Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Fri, Mar 19, 2021 at 09:20:16AM -0700, Shakeel Butt wrote: > One suggestion would be to make get_mem_cgroup_from_mm() more generic > (i.e. handle !mm && active_memcg() case) and avoid > get_mem_cgroup_from_current() as it might go away. Yeah, that occurred to me as well. I'll take a stab at doing that.
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Fri, Mar 19, 2021 at 8:51 AM Dan Schatzberg wrote: > > On Thu, Mar 18, 2021 at 05:56:28PM -0700, Shakeel Butt wrote: > > > > We need something similar for mem_cgroup_swapin_charge_page() as well. > > > > It is better to take this series in mm tree and Jens is ok with that [1]. > > > > [1] > > https://lore.kernel.org/linux-next/4fea89a5-0e18-0791-18a8-4c5907b0d...@kernel.dk/ > > It sounds like there are no concerns about the loop-related work in > the patch series. I'll rebase on the mm tree and resubmit. One suggestion would be to make get_mem_cgroup_from_mm() more generic (i.e. handle !mm && active_memcg() case) and avoid get_mem_cgroup_from_current() as it might go away.
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Thu, Mar 18, 2021 at 05:56:28PM -0700, Shakeel Butt wrote: > > We need something similar for mem_cgroup_swapin_charge_page() as well. > > It is better to take this series in mm tree and Jens is ok with that [1]. > > [1] > https://lore.kernel.org/linux-next/4fea89a5-0e18-0791-18a8-4c5907b0d...@kernel.dk/ It sounds like there are no concerns about the loop-related work in the patch series. I'll rebase on the mm tree and resubmit.
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Thu, Mar 18, 2021 at 4:46 PM Andrew Morton wrote:
>
> On Thu, 18 Mar 2021 10:00:17 -0600 Jens Axboe wrote:
>
> > On 3/18/21 9:53 AM, Shakeel Butt wrote:
> > > On Wed, Mar 17, 2021 at 3:30 PM Jens Axboe wrote:
> > >>
> > >> On 3/16/21 9:36 AM, Dan Schatzberg wrote:
> > >>> No major changes, just rebasing and resubmitting
> > >>
> > >> Applied for 5.13, thanks.
> > >>
> > >
> > > I have requested a couple of changes in the patch series. Can this
> > > applied series still be changed or new patches are required?
> >
> > I have nothing sitting on top of it for now, so as far as I'm concerned
> > we can apply a new series instead. Then we can also fold in that fix
> > from Colin that he posted this morning...
>
> The collision in memcontrol.c is a pain, but I guess as this is mainly
> a loop patch, the block tree is an appropriate route.
>
> Here's the collision between "mm: Charge active memcg when no mm is
> set" and Shakeels's
> https://lkml.kernel.org/r/20210305212639.775498-1-shake...@google.com
>
>
> --- mm/memcontrol.c
> +++ mm/memcontrol.c
> @@ -6728,8 +6730,15 @@ int mem_cgroup_charge(struct page *page, struct
> mm_struct *mm, gfp_t gfp_mask)
> rcu_read_unlock();
> }
>
> - if (!memcg)
> - memcg = get_mem_cgroup_from_mm(mm);
> + if (!memcg) {
> + if (!mm) {
> + memcg = get_mem_cgroup_from_current();
> + if (!memcg)
> + memcg = get_mem_cgroup_from_mm(current->mm);
> + } else {
> + memcg = get_mem_cgroup_from_mm(mm);
> + }
> + }
>
> ret = try_charge(memcg, gfp_mask, nr_pages);
> if (ret)
>
>
> Which I resolved thusly:
>
> int mem_cgroup_charge(struct page *page, struct mm_struct *mm, gfp_t gfp_mask)
> {
> struct mem_cgroup *memcg;
> int ret;
>
> if (mem_cgroup_disabled())
> return 0;
>
> if (!mm) {
> memcg = get_mem_cgroup_from_current();
> (!memcg)
> memcg = get_mem_cgroup_from_mm(current->mm);
> } else {
> memcg = get_mem_cgroup_from_mm(mm);
> }
>
> ret = __mem_cgroup_charge(page, memcg, gfp_mask);
> css_put(>css);
>
> return ret;
> }
>
We need something similar for mem_cgroup_swapin_charge_page() as well.
It is better to take this series in mm tree and Jens is ok with that [1].
[1]
https://lore.kernel.org/linux-next/4fea89a5-0e18-0791-18a8-4c5907b0d...@kernel.dk/
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Thu, 18 Mar 2021 10:00:17 -0600 Jens Axboe wrote:
> On 3/18/21 9:53 AM, Shakeel Butt wrote:
> > On Wed, Mar 17, 2021 at 3:30 PM Jens Axboe wrote:
> >>
> >> On 3/16/21 9:36 AM, Dan Schatzberg wrote:
> >>> No major changes, just rebasing and resubmitting
> >>
> >> Applied for 5.13, thanks.
> >>
> >
> > I have requested a couple of changes in the patch series. Can this
> > applied series still be changed or new patches are required?
>
> I have nothing sitting on top of it for now, so as far as I'm concerned
> we can apply a new series instead. Then we can also fold in that fix
> from Colin that he posted this morning...
The collision in memcontrol.c is a pain, but I guess as this is mainly
a loop patch, the block tree is an appropriate route.
Here's the collision between "mm: Charge active memcg when no mm is
set" and Shakeels's
https://lkml.kernel.org/r/20210305212639.775498-1-shake...@google.com
--- mm/memcontrol.c
+++ mm/memcontrol.c
@@ -6728,8 +6730,15 @@ int mem_cgroup_charge(struct page *page, struct
mm_struct *mm, gfp_t gfp_mask)
rcu_read_unlock();
}
- if (!memcg)
- memcg = get_mem_cgroup_from_mm(mm);
+ if (!memcg) {
+ if (!mm) {
+ memcg = get_mem_cgroup_from_current();
+ if (!memcg)
+ memcg = get_mem_cgroup_from_mm(current->mm);
+ } else {
+ memcg = get_mem_cgroup_from_mm(mm);
+ }
+ }
ret = try_charge(memcg, gfp_mask, nr_pages);
if (ret)
Which I resolved thusly:
int mem_cgroup_charge(struct page *page, struct mm_struct *mm, gfp_t gfp_mask)
{
struct mem_cgroup *memcg;
int ret;
if (mem_cgroup_disabled())
return 0;
if (!mm) {
memcg = get_mem_cgroup_from_current();
(!memcg)
memcg = get_mem_cgroup_from_mm(current->mm);
} else {
memcg = get_mem_cgroup_from_mm(mm);
}
ret = __mem_cgroup_charge(page, memcg, gfp_mask);
css_put(>css);
return ret;
}
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On 3/18/21 9:53 AM, Shakeel Butt wrote: > On Wed, Mar 17, 2021 at 3:30 PM Jens Axboe wrote: >> >> On 3/16/21 9:36 AM, Dan Schatzberg wrote: >>> No major changes, just rebasing and resubmitting >> >> Applied for 5.13, thanks. >> > > I have requested a couple of changes in the patch series. Can this > applied series still be changed or new patches are required? I have nothing sitting on top of it for now, so as far as I'm concerned we can apply a new series instead. Then we can also fold in that fix from Colin that he posted this morning... -- Jens Axboe
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On Wed, Mar 17, 2021 at 3:30 PM Jens Axboe wrote: > > On 3/16/21 9:36 AM, Dan Schatzberg wrote: > > No major changes, just rebasing and resubmitting > > Applied for 5.13, thanks. > I have requested a couple of changes in the patch series. Can this applied series still be changed or new patches are required?
Re: [PATCH v10 0/3] Charge loop device i/o to issuing cgroup
On 3/16/21 9:36 AM, Dan Schatzberg wrote: > No major changes, just rebasing and resubmitting Applied for 5.13, thanks. -- Jens Axboe
