On Wed, Nov 7, 2012 at 7:24 AM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > checkpatch finds a number of problems with this patch, all of which > should be fixed. Please always use checkpatch. Sorry for missing the check. >> + /* only clear the flag for one device if all >> + * children of the device don't set the flag. >> + */ > > Such a comment is usually laid out as > > /* > * Only ... Will do it in -v5. > More significantly, the comment describes what the code is doing but > not why the code is doing it. The former is (usually) obvious from > reading the C, and the latter is what good code comments address. > > And it's needed in this case. Why does the code do this? Suppose both two usb scsi disks which share the same usb configuration(device) set the device memalloc_noio flag, and its ancestors' memalloc_noio flag should be cleared only after both the two usb scsi disk's flags have been cleared. OK, we'll add comment on clearing flag. > > Also, can a device have more than one child? If so, the code doesn't > do what the comment says it does. It should do that because device_for_each_child() returns true immediately only if dev_memalloc_noio() for one child returns true. > >> + if (!dev || (!enable && >> + device_for_each_child(dev, NULL, >> + dev_memalloc_noio))) >> + break; >> + } >> + mutex_unlock(&dev_hotplug_mutex); >> +} >> +EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio); Thanks, -- Ming Lei -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>