Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-15 Thread Richard Weinberger
Am Samstag, 15. Dezember 2018, 11:51:12 CET schrieb Christoph Hellwig: > FYI, for iomap we got a patch to just increment the page count when > setting the private data, and it finally got merged into mainline after > a while. > > Not that it totally makes sense to me, but it is what it is. It wou

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-15 Thread Christoph Hellwig
FYI, for iomap we got a patch to just increment the page count when setting the private data, and it finally got merged into mainline after a while. Not that it totally makes sense to me, but it is what it is. It would just be nice if set_page_private took care of it and we had a clear_page_priva

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-14 Thread Gao Xiang
On 2018/12/14 13:56, zhangjun wrote: > IOMAP uses PG_private a little different with buffer_head based > filesystem. > It uses it as marker and when set, the page counter is not incremented, > migrate_page_move_mapping() assumes that PG_private indicates a counter > of +1. > so, we have to pass a

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-14 Thread Gao Xiang
Hi Richard, On 2018/12/14 21:35, Richard Weinberger wrote: > Hmm, in case of UBIFS it seems easy. We can add a get/put_page() around > setting/clearing > the flag. > I did that now and so far none of my tests exploded. Yes, many existed codes are based on this restriction in order to be freeable

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-14 Thread Richard Weinberger
Am Freitag, 14. Dezember 2018, 13:26:28 CET schrieb Gao Xiang: > Hi Richard, > > On 2018/12/14 19:25, Richard Weinberger wrote: > > This is the third place which needs this workaround. > > UBIFS, F2FS, and now iomap. > > > > I agree with Dave that nobody can assume that PG_private implies an > >

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-14 Thread Gao Xiang
Hi Richard, On 2018/12/14 19:25, Richard Weinberger wrote: > This is the third place which needs this workaround. > UBIFS, F2FS, and now iomap. > > I agree with Dave that nobody can assume that PG_private implies an additional > page reference. > But page migration does that. Including parts of t

Re: [PATCH] fix page_count in ->iomap_migrate_page()

2018-12-14 Thread Richard Weinberger
[CC'ing authors of the code plus mm folks] Am Freitag, 14. Dezember 2018, 06:56:01 CET schrieb zhangjun: > IOMAP uses PG_private a little different with buffer_head based > filesystem. > It uses it as marker and when set, the page counter is not incremented, > migrate_page_move_mapping() assumes t

[PATCH] fix page_count in ->iomap_migrate_page()

2018-12-13 Thread zhangjun
IOMAP uses PG_private a little different with buffer_head based filesystem. It uses it as marker and when set, the page counter is not incremented, migrate_page_move_mapping() assumes that PG_private indicates a counter of +1. so, we have to pass a extra count of -1 to migrate_page_move_mapping() i