| Message ID | 20200528135444.11508-1-schatzberg.dan@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Charge loop device i/o to issuing cgroup | expand |
On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > Much of the discussion about this has died down. There's been a > concern raised that we could generalize infrastructure across loop, > md, etc. This may be possible, in the future, but it isn't clear to me > how this would look like. I'm inclined to fix the existing issue with > loop devices now (this is a problem we hit at FB) and address > consolidation with other cases if and when those need to be addressed. > What's the status of this series?
On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: > On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > > > Much of the discussion about this has died down. There's been a > > concern raised that we could generalize infrastructure across loop, > > md, etc. This may be possible, in the future, but it isn't clear to me > > how this would look like. I'm inclined to fix the existing issue with > > loop devices now (this is a problem we hit at FB) and address > > consolidation with other cases if and when those need to be addressed. > > > > What's the status of this series? Thanks for reminding me about this. I haven't got any further feedback. I'll bug Jens to take a look and see if he has any concerns and if not send a rebased version.
On 8/21/20 9:04 AM, Dan Schatzberg wrote: > On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: >> On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: >>> >>> Much of the discussion about this has died down. There's been a >>> concern raised that we could generalize infrastructure across loop, >>> md, etc. This may be possible, in the future, but it isn't clear to me >>> how this would look like. I'm inclined to fix the existing issue with >>> loop devices now (this is a problem we hit at FB) and address >>> consolidation with other cases if and when those need to be addressed. >>> >> >> What's the status of this series? > > Thanks for reminding me about this. I haven't got any further > feedback. I'll bug Jens to take a look and see if he has any concerns > and if not send a rebased version. No immediate concerns, I think rebasing and sending one against the current tree is probably a good idea. Then we can hopefully get it queued up for 5.10.
On Fri, Aug 21, 2020 at 11:04:05AM -0400, Dan Schatzberg wrote: > On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: > > On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > > > > > Much of the discussion about this has died down. There's been a > > > concern raised that we could generalize infrastructure across loop, > > > md, etc. This may be possible, in the future, but it isn't clear to me > > > how this would look like. I'm inclined to fix the existing issue with > > > loop devices now (this is a problem we hit at FB) and address > > > consolidation with other cases if and when those need to be addressed. > > > > > > > What's the status of this series? > > Thanks for reminding me about this. I haven't got any further > feedback. I'll bug Jens to take a look and see if he has any concerns > and if not send a rebased version. Just as a note, I stole a patch from this series called "mm: support nesting memalloc_use_memcg()" to use for the bpf memory accounting. I rewrote the commit log and rebased to the tot with some trivial changes. I just sent it upstream: https://lore.kernel.org/bpf/20200821150134.2581465-1-guro@fb.com/T/#md7edb6b5b940cee1c4d15e3cef17aa8b07328c2e It looks like we need it for two independent sub-systems, so I wonder if we want to route it first through the mm tree as a standalone patch? Thanks!
On Fri, Aug 21, 2020 at 9:02 AM Roman Gushchin <guro@fb.com> wrote: > > On Fri, Aug 21, 2020 at 11:04:05AM -0400, Dan Schatzberg wrote: > > On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: > > > On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > > > > > > > Much of the discussion about this has died down. There's been a > > > > concern raised that we could generalize infrastructure across loop, > > > > md, etc. This may be possible, in the future, but it isn't clear to me > > > > how this would look like. I'm inclined to fix the existing issue with > > > > loop devices now (this is a problem we hit at FB) and address > > > > consolidation with other cases if and when those need to be addressed. > > > > > > > > > > What's the status of this series? > > > > Thanks for reminding me about this. I haven't got any further > > feedback. I'll bug Jens to take a look and see if he has any concerns > > and if not send a rebased version. > > Just as a note, I stole a patch from this series called > "mm: support nesting memalloc_use_memcg()" to use for the bpf memory accounting. > I rewrote the commit log and rebased to the tot with some trivial changes. > > I just sent it upstream: > https://lore.kernel.org/bpf/20200821150134.2581465-1-guro@fb.com/T/#md7edb6b5b940cee1c4d15e3cef17aa8b07328c2e > > It looks like we need it for two independent sub-systems, so I wonder > if we want to route it first through the mm tree as a standalone patch? > Another way is to push that patch to 5.9-rc2 linus tree, so both block and mm branches for 5.10 will have it. (Not sure if that's ok.)
On Fri, Aug 21, 2020 at 09:27:56AM -0700, Shakeel Butt wrote: > On Fri, Aug 21, 2020 at 9:02 AM Roman Gushchin <guro@fb.com> wrote: > > > > On Fri, Aug 21, 2020 at 11:04:05AM -0400, Dan Schatzberg wrote: > > > On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: > > > > On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > > > > > > > > > Much of the discussion about this has died down. There's been a > > > > > concern raised that we could generalize infrastructure across loop, > > > > > md, etc. This may be possible, in the future, but it isn't clear to me > > > > > how this would look like. I'm inclined to fix the existing issue with > > > > > loop devices now (this is a problem we hit at FB) and address > > > > > consolidation with other cases if and when those need to be addressed. > > > > > > > > > > > > > What's the status of this series? > > > > > > Thanks for reminding me about this. I haven't got any further > > > feedback. I'll bug Jens to take a look and see if he has any concerns > > > and if not send a rebased version. > > > > Just as a note, I stole a patch from this series called > > "mm: support nesting memalloc_use_memcg()" to use for the bpf memory accounting. > > I rewrote the commit log and rebased to the tot with some trivial changes. > > > > I just sent it upstream: > > https://lore.kernel.org/bpf/20200821150134.2581465-1-guro@fb.com/T/#md7edb6b5b940cee1c4d15e3cef17aa8b07328c2e > > > > It looks like we need it for two independent sub-systems, so I wonder > > if we want to route it first through the mm tree as a standalone patch? > > > > Another way is to push that patch to 5.9-rc2 linus tree, so both block > and mm branches for 5.10 will have it. (Not sure if that's ok.) Ok, it looks like the patch provides a generally useful API enhancement. And we do have at least two potential use cases for it. Let me send it as a standalone patch to linux-mm@. Btw, Shakeel, what do you think of s/memalloc_use_memcg()/set_active_memcg() ? And thank you for reviews! Roman
On Fri, Aug 21, 2020 at 1:05 PM Roman Gushchin <guro@fb.com> wrote: > > On Fri, Aug 21, 2020 at 09:27:56AM -0700, Shakeel Butt wrote: > > On Fri, Aug 21, 2020 at 9:02 AM Roman Gushchin <guro@fb.com> wrote: > > > > > > On Fri, Aug 21, 2020 at 11:04:05AM -0400, Dan Schatzberg wrote: > > > > On Thu, Aug 20, 2020 at 10:06:44AM -0700, Shakeel Butt wrote: > > > > > On Thu, May 28, 2020 at 6:55 AM Dan Schatzberg <schatzberg.dan@gmail.com> wrote: > > > > > > > > > > > > Much of the discussion about this has died down. There's been a > > > > > > concern raised that we could generalize infrastructure across loop, > > > > > > md, etc. This may be possible, in the future, but it isn't clear to me > > > > > > how this would look like. I'm inclined to fix the existing issue with > > > > > > loop devices now (this is a problem we hit at FB) and address > > > > > > consolidation with other cases if and when those need to be addressed. > > > > > > > > > > > > > > > > What's the status of this series? > > > > > > > > Thanks for reminding me about this. I haven't got any further > > > > feedback. I'll bug Jens to take a look and see if he has any concerns > > > > and if not send a rebased version. > > > > > > Just as a note, I stole a patch from this series called > > > "mm: support nesting memalloc_use_memcg()" to use for the bpf memory accounting. > > > I rewrote the commit log and rebased to the tot with some trivial changes. > > > > > > I just sent it upstream: > > > https://lore.kernel.org/bpf/20200821150134.2581465-1-guro@fb.com/T/#md7edb6b5b940cee1c4d15e3cef17aa8b07328c2e > > > > > > It looks like we need it for two independent sub-systems, so I wonder > > > if we want to route it first through the mm tree as a standalone patch? > > > > > > > Another way is to push that patch to 5.9-rc2 linus tree, so both block > > and mm branches for 5.10 will have it. (Not sure if that's ok.) > > Ok, it looks like the patch provides a generally useful API enhancement. > And we do have at least two potential use cases for it. > Let me send it as a standalone patch to linux-mm@. > > Btw, Shakeel, what do you think of s/memalloc_use_memcg()/set_active_memcg() ? > I am fine with it.
Much of the discussion about this has died down. There's been a concern raised that we could generalize infrastructure across loop, md, etc. This may be possible, in the future, but it isn't clear to me how this would look like. I'm inclined to fix the existing issue with loop devices now (this is a problem we hit at FB) and address consolidation with other cases if and when those need to be addressed. Changes since V6: * Added separate spinlock for worker synchronization * Minor style changes Changes since V5: * Fixed a missing css_put when failing to allocate a worker * Minor style changes Changes since V4: Only patches 1 and 2 have changed. * Fixed irq lock ordering bug * Simplified loop detach * Added support for nesting memalloc_use_memcg Changes since V3: * Fix race on loop device destruction and deferred worker cleanup * Ensure charge on shmem_swapin_page works just like getpage * Minor style changes Changes since V2: * Deferred destruction of workqueue items so in the common case there is no allocation needed Changes since V1: * Split out and reordered patches so cgroup charging changes are separate from kworker -> workqueue change * Add mem_css to struct loop_cmd to simplify logic The loop device runs all i/o to the backing file on a separate kworker thread which results in all i/o being charged to the root cgroup. This allows a loop device to be used to trivially bypass resource limits and other policy. This patch series fixes this gap in accounting. A simple script to demonstrate this behavior on cgroupv2 machine: ''' #!/bin/bash set -e CGROUP=/sys/fs/cgroup/test.slice LOOP_DEV=/dev/loop0 if [[ ! -d $CGROUP ]] then sudo mkdir $CGROUP fi grep oom_kill $CGROUP/memory.events # Set a memory limit, write more than that limit to tmpfs -> OOM kill sudo unshare -m bash -c " echo \$\$ > $CGROUP/cgroup.procs; echo 0 > $CGROUP/memory.swap.max; echo 64M > $CGROUP/memory.max; mount -t tmpfs -o size=512m tmpfs /tmp; dd if=/dev/zero of=/tmp/file bs=1M count=256" || true grep oom_kill $CGROUP/memory.events # Set a memory limit, write more than that limit through loopback # device -> no OOM kill sudo unshare -m bash -c " echo \$\$ > $CGROUP/cgroup.procs; echo 0 > $CGROUP/memory.swap.max; echo 64M > $CGROUP/memory.max; mount -t tmpfs -o size=512m tmpfs /tmp; truncate -s 512m /tmp/backing_file losetup $LOOP_DEV /tmp/backing_file dd if=/dev/zero of=$LOOP_DEV bs=1M count=256; losetup -D $LOOP_DEV" || true grep oom_kill $CGROUP/memory.events ''' Naively charging cgroups could result in priority inversions through the single kworker thread in the case where multiple cgroups are reading/writing to the same loop device. This patch series does some minor modification to the loop driver so that each cgroup can make forward progress independently to avoid this inversion. With this patch series applied, the above script triggers OOM kills when writing through the loop device as expected. Dan Schatzberg (3): loop: Use worker per cgroup instead of kworker mm: Charge active memcg when no mm is set loop: Charge i/o to mem and blk cg Johannes Weiner (1): mm: support nesting memalloc_use_memcg() drivers/block/loop.c | 244 ++++++++++++++++++++++----- drivers/block/loop.h | 15 +- fs/buffer.c | 6 +- fs/notify/fanotify/fanotify.c | 5 +- fs/notify/inotify/inotify_fsnotify.c | 5 +- include/linux/memcontrol.h | 6 + include/linux/sched/mm.h | 28 +-- kernel/cgroup/cgroup.c | 1 + mm/memcontrol.c | 11 +- mm/shmem.c | 4 +- 10 files changed, 246 insertions(+), 79 deletions(-)