Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434537 > unrolled thread

Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock Peter Zijlstra <peterz@infradead.org> - 2016-06-30 15:10 +0200
    Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock Byungchul Park <byungchul.park@lge.com> - 2016-07-01 01:30 +0200
      Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock Peter Zijlstra <peterz@infradead.org> - 2016-07-01 10:20 +0200
      Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-07-01 13:30 +0200
        Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock Byungchul Park <byungchul.park@lge.com> - 2016-07-04 06:40 +0200

#1434537 — Re: [RFC 08/12] lockdep: Apply crossrelease to PG_locked lock

On Mon, Jun 20, 2016 at 01:55:23PM +0900, Byungchul Park wrote:
> @@ -215,6 +219,11 @@ struct page {
>  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
>  	int _last_cpupid;
>  #endif
> +
> +#ifdef CONFIG_LOCKDEP_PAGELOCK
> +	struct lockdep_map map;
> +	struct cross_lock xlock;
> +#endif
>  }

So that's 32+64=96 bytes (CONFIG_LOCK_STAT=n) added to struct page,
really!?

[toc] | [next] | [standalone]

#1434906

On Thu, Jun 30, 2016 at 03:04:58PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 20, 2016 at 01:55:23PM +0900, Byungchul Park wrote:
> > @@ -215,6 +219,11 @@ struct page {
> >  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
> >  	int _last_cpupid;
> >  #endif
> > +
> > +#ifdef CONFIG_LOCKDEP_PAGELOCK
> > +	struct lockdep_map map;
> > +	struct cross_lock xlock;
> > +#endif
> >  }
> 
> So that's 32+64=96 bytes (CONFIG_LOCK_STAT=n) added to struct page,
> really!?

Yes... I concerned it at first, but I thought it would be ok since
CONFIG_LOCKDEP_PAGE is a debug feature. Anyway, I will try to reduce
the size of struct cross_lock which is only thing I can do to reduce
it, since we cannot avoid using lockdep_map if we want to make
lock_page() participate in the lockdep play.

[toc] | [prev] | [next] | [standalone]

#1435123

On Fri, Jul 01, 2016 at 08:21:21AM +0900, Byungchul Park wrote:
> On Thu, Jun 30, 2016 at 03:04:58PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 20, 2016 at 01:55:23PM +0900, Byungchul Park wrote:
> > > @@ -215,6 +219,11 @@ struct page {
> > >  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
> > >  	int _last_cpupid;
> > >  #endif
> > > +
> > > +#ifdef CONFIG_LOCKDEP_PAGELOCK
> > > +	struct lockdep_map map;
> > > +	struct cross_lock xlock;
> > > +#endif
> > >  }
> > 
> > So that's 32+64=96 bytes (CONFIG_LOCK_STAT=n) added to struct page,
> > really!?
> 
> Yes... I concerned it at first, but I thought it would be ok since
> CONFIG_LOCKDEP_PAGE is a debug feature.

Right, but still, that's 0.75 GB of memory on my desktop (32GB total)
just for a debug feature. It grows struct page from 1.5% to 3.9% of
total memory, that is immense.

We've avoided doing this for ptl; which was doable because typically
only a small number of pages ends up being a pagetable.

In any case, I feel PG_locked is special enough to fudge. After all, the
content of all these lockdep_map thingies would basically be the same,
which is a massive waste of space.

I still need to bend my brain around this xlock stuff, that just didn't
want to parse when I looked at it last night.

[toc] | [prev] | [next] | [standalone]

#1435264

On Fri, Jul 01, 2016 at 08:21:21AM +0900, Byungchul Park wrote:
> On Thu, Jun 30, 2016 at 03:04:58PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 20, 2016 at 01:55:23PM +0900, Byungchul Park wrote:
> > > @@ -215,6 +219,11 @@ struct page {
> > >  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
> > >  	int _last_cpupid;
> > >  #endif
> > > +
> > > +#ifdef CONFIG_LOCKDEP_PAGELOCK
> > > +	struct lockdep_map map;
> > > +	struct cross_lock xlock;
> > > +#endif
> > >  }
> > 
> > So that's 32+64=96 bytes (CONFIG_LOCK_STAT=n) added to struct page,
> > really!?
> 
> Yes... I concerned it at first, but I thought it would be ok since
> CONFIG_LOCKDEP_PAGE is a debug feature. Anyway, I will try to reduce
> the size of struct cross_lock which is only thing I can do to reduce
> it, since we cannot avoid using lockdep_map if we want to make
> lock_page() participate in the lockdep play.

Please use page_ext instead. With boottime switch to enable.

This way we can have this compile-time debug option enabled on more
machines without unnecessary runtime overhead.

And, please, CC linux-mm next time.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1436453

On Fri, Jul 01, 2016 at 02:18:46PM +0300, Kirill A. Shutemov wrote:
> On Fri, Jul 01, 2016 at 08:21:21AM +0900, Byungchul Park wrote:
> > On Thu, Jun 30, 2016 at 03:04:58PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jun 20, 2016 at 01:55:23PM +0900, Byungchul Park wrote:
> > > > @@ -215,6 +219,11 @@ struct page {
> > > >  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
> > > >  	int _last_cpupid;
> > > >  #endif
> > > > +
> > > > +#ifdef CONFIG_LOCKDEP_PAGELOCK
> > > > +	struct lockdep_map map;
> > > > +	struct cross_lock xlock;
> > > > +#endif
> > > >  }
> > > 
> > > So that's 32+64=96 bytes (CONFIG_LOCK_STAT=n) added to struct page,
> > > really!?
> > 
> > Yes... I concerned it at first, but I thought it would be ok since
> > CONFIG_LOCKDEP_PAGE is a debug feature. Anyway, I will try to reduce
> > the size of struct cross_lock which is only thing I can do to reduce
> > it, since we cannot avoid using lockdep_map if we want to make
> > lock_page() participate in the lockdep play.
> 
> Please use page_ext instead. With boottime switch to enable.
> 
> This way we can have this compile-time debug option enabled on more
> machines without unnecessary runtime overhead.

Thank you for advice.

I also think it's one of good candidates except the fact that it have to
depend on page_ext additionally.

> 
> And, please, CC linux-mm next time.
> 
> -- 
>  Kirill A. Shutemov

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web