On Wed 17-04-19 15:24:47, David Hildenbrand wrote: > On 17.04.19 15:12, Michal Hocko wrote: > > On Tue 09-04-19 12:01:45, David Hildenbrand wrote: > >> __add_pages() doesn't add the memory resource, so __remove_pages() > >> shouldn't remove it. Let's factor it out. Especially as it is a special > >> case for memory used as system memory, added via add_memory() and > >> friends. > >> > >> We now remove the resource after removing the sections instead of doing > >> it the other way around. I don't think this change is problematic. > >> > >> add_memory() > >> register memory resource > >> arch_add_memory() > >> > >> remove_memory > >> arch_remove_memory() > >> release memory resource > >> > >> While at it, explain why we ignore errors and that it only happeny if > >> we remove memory in a different granularity as we added it. > > > > OK, I agree that the symmetry is good in general and it certainly makes > > sense here as well. But does it make sense to pick up this particular > > part without larger considerations of add vs. remove apis? I have a > > strong feeling this wouldn't be the only thing to care about. In other > > words does this help future changes or it is more likely to cause more > > code conflicts with other features being developed right now? > > I am planning to > > 1. factor out memory block device handling, so features like sub-section > add/remove are easier to add internally. Move it to the user that wants > it. Clean up all the mess we have right now due to supporting memory > block devices that span several sections. > > 2. Make sure that any arch_add_pages() and friends clean up properly if > they fail instead of indicating failure but leaving some partially added > memory lying around. > > 3. Clean up node handling regarding to memory hotplug/unplug. Especially > don't allow to offline/remove memory spanning several nodes etc. Yes, this all sounds sane to me. > IOW, in order to properly clean up memory block device handling and > prepare for more changes people are interested in (e.g. sub-section add > of device memory), this is the right thing to do. The other parts are > bigger changes. This would be really valuable to have in the cover. Beause there is so much to clean up in this mess but making random small cleanups without a larger plan tends to step on others toes more than being useful. -- Michal Hocko SUSE Labs
