On Mon, 25 Sep 2023, Matthew Wilcox wrote: > On Mon, Sep 25, 2023 at 01:29:28AM -0700, Hugh Dickins wrote: > > + /* alloc node covering entire file; adds ref to new */ > > This comment is confusing. sp_alloc initialises the refcount of 'n' to 1. > Which is the same memory referred to by the name 'new' in __mpol_dup(), > but in this function, the name "new" refers to the mempolicy called > "old" in __mpol_dup(). No promises, but I'll see if I can make it look better in v2. > > > + n = sp_alloc(0, MAX_LFS_FILESIZE >> PAGE_SHIFT, new); > > + if (n) > > + sp_insert(sp, n); > > put_new: > > mpol_put(new); /* drop initial ref */ > > free_scratch: > > This is all a bit inefficient, really. We call mpol_new() to get a > new mpol, then we set it up, then we dup it, then we free it. It'd > be nice if we could donate it instead of copying it. Maybe you'll > do something like that later. "later" is probably the operative word. I do have an unincluded 2017 patch where I had that same realization, and wrote "I suspect that this series of commits may be adding to an absurdity of over-mpol_dup()ing: but that's for some other future cleanup, right now I'm just happy not to be corrupting or leaking mpols." Hugh
