On 2/6/23 6:03?PM, Alistair Popple wrote: > > Jens Axboe <axboe@xxxxxxxxx> writes: > >> On 2/6/23 12:47?AM, Alistair Popple wrote: >>> Convert io_uring to use vm_account instead of directly charging pages >>> against the user/mm. Rather than charge pages to both user->locked_vm >>> and mm->pinned_vm this will only charge pages to user->locked_vm. >> >> Not sure how we're supposed to review this, when you just send us 9/19 >> and vm_account_release() is supposedly an earlier patch in this series. >> >> Either CC the whole series, or at least the cover letter, core parts, >> and the per-subsystem parts. > > Ok, thanks. Will be sure to add everyone to the cover letter and patch > 01 when I send the next version. > > For reference the cover letter is here: > > https://lore.kernel.org/linux-mm/cover.c238416f0e82377b449846dbb2459ae9d7030c8e.1675669136.git-series.apopple@xxxxxxxxxx/ > > And the core patch that introduces vm_account is here: > > https://lore.kernel.org/linux-mm/e80b61561f97296a6c08faeebe281cb949333d1d.1675669136.git-series.apopple@xxxxxxxxxx/ > > No problem if you want to wait for the resend/next version before > taking another look though. Thanks, that helps. Like listed in the cover letter, I also have to agree that this is badly named. It's way too generic, it needs to have a name that tells you what it does. There's tons of accounting, you need to be more specific. Outside of that, we're now doubling the amount of memory associated with tracking this. That isn't necessarily a showstopper, but it is not ideal. I didn't take a look at the other conversions (again, because they were not sent to me), but seems like the task_struct and flags could just be passed in as they may very well be known to many/most callers? -- Jens Axboe
