Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 03:19:46PM +0900, Byungchul Park wrote:
> On 12/5/2017 2:46 PM, Byungchul Park wrote:
> > On 12/5/2017 2:30 PM, Matthew Wilcox wrote:
> > > On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
> > > > For now, wait_for_completion() / complete() works with lockdep, add
> > > > lock_page() / unlock_page() and its family to lockdep support.
> > > > 
> > > > Changes from v1
> > > >   - Move lockdep_map_cross outside of page_ext to make it flexible
> > > >   - Prevent allocating lockdep_map per page by default
> > > >   - Add a boot parameter allowing the allocation for debugging
> > > > 
> > > > Byungchul Park (4):
> > > >    lockdep: Apply crossrelease to PG_locked locks
> > > >    lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
> > > >    lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
> > > >    lockdep: Add a boot parameter enabling to track page locks using
> > > >      lockdep and disable it by default
> > > 
> > > I don't like the way you've structured this patch series; first adding
> > > the lockdep map to struct page, then moving it to page_ext.
> > 
> > Hello,
> > 
> > I will make them into one patch.
> 
> I've thought it more.
> 
> Actually I did it because I thought I'd better make it into two
> patches since it makes reviewers easier to review. It doesn't matter
> which one I choose, but I prefer to split it.

I don't know whether it's better to make it all one patch or split it
into multiple patches.  But it makes no sense to introduce it in struct
page, then move it to struct page_ext.