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

On Mon 04-12-17 23:45:17, Matthew Wilcox wrote:

On Tue, Dec 05, 2017 at 03:19:46PM +0900, Byungchul Park wrote:

quoted

On 12/5/2017 2:46 PM, Byungchul Park wrote:

quoted

On 12/5/2017 2:30 PM, Matthew Wilcox wrote:

quoted

On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:

quoted

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.

I would tend to agree. It is not like anybody would like to apply only
the first part alone. Adding the necessary infrastructure to page_ext is
not such a big deal.

-- 
Michal Hocko
SUSE Labs