On Tue 07-04-20 17:31:33, Yafang Shao wrote: > On Tue, Apr 7, 2020 at 2:43 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > > > [I have only now noticed there was another version posted. Please try to > > wait a bit longer for other feedback before reposting a newer version] > > > > Sure I will. sorry about that. > But after we send a patch, we always don't know in which state this > patch is, e.g. Changes Requested, Not Applicable, Rejected, Needs > Review / ACK and etc, as listed in netdev list[1]. That could give us > an explicit instruction on what to do next. Yes I can see how that can be unclear or consuming at times. But a general rule of thumb I would suggest is to simply wait few days before reposting a new version. If you need to discuss an incremental change based on the review feedback you can always do that in the original email thread and repost the new version after the review feedback settles. > [1]. https://patchwork.ozlabs.org/project/netdev/list/?state=*&page=2¶m=3 > > > On Tue 07-04-20 11:11:13, Yafang Shao wrote: [...] > > See my comment about EBUSY in the previous version > > http://lkml.kernel.org/r/20200407063621.GA18914@xxxxxxxxxxxxxx > > > > >From the perspective of mkdir(2), it is better to use ENOSPC here. But > I'm not sure whether it is better to update the man page of mkdir(2) > or not. Well, I do not know whether EBUSY missing in the man page is an omission or an intention. But adding it only for this single operation failure would sound like an overkill to me. > I checked the explaination about ENOSPC in 73f576c04b94 ("mm: > memcontrol: fix cgroup creation failure after many small jobs") > carefully, but I don't a clear idea which one is better now. The changelog simply mentioned that without the additional id tracking the ENOSPC would have been returned from elsewhere. I haven't checked but I suspect it would be from the cgroup core. This patch just didn't propagate the idr failure specifically and hid it under the ENOMEM failure path. I can only speculate why that was the case but I suspect that Johannes simply didn't consider the distinction important enough. All I am saying is that preserving the failure mode would be preferable. Especially unless there a strong reason to use EBUSY which then should be carefully explained in the changelog. -- Michal Hocko SUSE Labs
