[LSF/MM TOPIC] Killing reliance on struct page->mapping

[LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

I started a patchset about $TOPIC a while ago, right now i am working on other
thing but i hope to have an RFC for $TOPIC before LSF/MM and thus would like a
slot during common track to talk about it as it impacts FS, BLOCK and MM (i am
assuming their will be common track).

Idea is that mapping (struct address_space) is available in virtualy all the
places where it is needed and that their should be no reasons to depend only on
struct page->mapping field. My patchset basicly add mapping to a bunch of vfs
callback (struct address_space_operations) where it is missing, changing call
site. Then i do an individual patch per filesystem to leverage the new argument
instead on struct page.

I am doing this for a generic page write protection mechanism which generalize
KSM to file back page. They are couple other aspect like struct page->index,
struct page->private which are addressed in similar way. The block layer is
mostly affected because on block device error it needs the page->mapping to
report I/O error.

Maybe we can kill page->mapping altogether as a result of this. However this is
not my motivation at this time.


Sorry for absence of patchset at this time but i wanted to submit the subject
before LSF/MM deadline.

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Al Viro]

On Mon, Jan 29, 2018 at 07:43:48PM -0500, Jerome Glisse wrote:
> I started a patchset about $TOPIC a while ago, right now i am working on other
> thing but i hope to have an RFC for $TOPIC before LSF/MM and thus would like a
> slot during common track to talk about it as it impacts FS, BLOCK and MM (i am
> assuming their will be common track).
> 
> Idea is that mapping (struct address_space) is available in virtualy all the
> places where it is needed and that their should be no reasons to depend only on
> struct page->mapping field. My patchset basicly add mapping to a bunch of vfs
> callback (struct address_space_operations) where it is missing, changing call
> site. Then i do an individual patch per filesystem to leverage the new argument
> instead on struct page.

Oh?  What about the places like fs/coda?  Or block devices, for that matter...
You can't count upon file->f_mapping->host == file_inode(file).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Wed, Jan 31, 2018 at 04:56:46PM +0000, Al Viro wrote:
> On Mon, Jan 29, 2018 at 07:43:48PM -0500, Jerome Glisse wrote:
> > I started a patchset about $TOPIC a while ago, right now i am working on other
> > thing but i hope to have an RFC for $TOPIC before LSF/MM and thus would like a
> > slot during common track to talk about it as it impacts FS, BLOCK and MM (i am
> > assuming their will be common track).
> > 
> > Idea is that mapping (struct address_space) is available in virtualy all the
> > places where it is needed and that their should be no reasons to depend only on
> > struct page->mapping field. My patchset basicly add mapping to a bunch of vfs
> > callback (struct address_space_operations) where it is missing, changing call
> > site. Then i do an individual patch per filesystem to leverage the new argument
> > instead on struct page.
> 
> Oh?  What about the places like fs/coda?  Or block devices, for that matter...
> You can't count upon file->f_mapping->host == file_inode(file).

What matter is that the place that call an address_space_operations callback
already has mapping == page->mapping in many places this is obvious. For
instance page just have been looked up using mapping and thus you must have
mapping == page->mapping. But i believe this holds in all places. They are
few dark corners (fuse, splice, ...). Truncate also factor in all this as
page->mapping is use to determine if a page has been truncated, but it
should not be an issue.

So i am not counting on file->f_mapping->host == file_inode(file) but i might
count in _some_ place on vma->file->f_mapping == page->mapping of any non private
page inside that vma. AFAICT this holds for coda and should hold elsewhere too.

For block devices the idea is to use struct page and buffer_head (first one of
a page) as a key to find mapping (struct address_space) back.

The overall idea i have is that in any place in the kernel (except memory reclaim
but that's ok) we can either get mapping or buffer_head information without relying
on struct page and if we have either one and a struct page then we can find the
other one.

Like i said i am not done with a patchset for that yet so maybe i am too
optimistic. I have another patchset i need to finish first before i go back to
this. I hope to have an RFC sometime in February or March and maybe by then
i would have found a roadblock, i am crossing my fingers until then :)

If it turns out that it is not doable i will comment on this thread and we can
kill that of from the agenda.

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Al Viro]

On Wed, Jan 31, 2018 at 12:42:45PM -0500, Jerome Glisse wrote:

> For block devices the idea is to use struct page and buffer_head (first one of
> a page) as a key to find mapping (struct address_space) back.

Details, please...

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Wed, Jan 31, 2018 at 05:55:58PM +0000, Al Viro wrote:
> On Wed, Jan 31, 2018 at 12:42:45PM -0500, Jerome Glisse wrote:
> 
> > For block devices the idea is to use struct page and buffer_head (first one of
> > a page) as a key to find mapping (struct address_space) back.
> 
> Details, please...

Note that i am not talking about block device page (i am excluding
those from that). So just regular filesystem page (ext*,xfs,btrfs,
...).

So in block device context AFAIK only time when you need mapping is
if they are some I/O error. Given than i am doing this with intent to
write protect the page one can argue that i can wait for all writeback
to complete before proceeding. At that time, it does not matter to block
device if page->mapping is no longer an address_space because the block
device code is done with the page and has forget about it.

That's one solution, another one is to have struct bio_vec store
buffer_head pointer and not page pointer, from buffer_head you can
find struct page and using buffer_head and struct page pointer you
can walk the KSM rmap_item chain to find back the mapping. This
would be needed on I/O error for pending writeback of a newly write
protected page, so one can argue that the overhead of the chain lookup
to find back the mapping against which to report IO error, is an
acceptable cost.

Another solution is to override the writeback end callback with
special one capable of finding the mapping from struct page and bio
pointer. This would not need any change to block device code. It
would have the same overhead thought as solution 2 above.


My intention was to stick to first solution (wait for writeback and
make no modification to block device struct or function). Then latter
if it make sense to add support to write protect a page before write
back is done.

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Lsf-pc] [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jens Axboe]

On 1/31/18 11:13 AM, Jerome Glisse wrote:
> That's one solution, another one is to have struct bio_vec store
> buffer_head pointer and not page pointer, from buffer_head you can
> find struct page and using buffer_head and struct page pointer you
> can walk the KSM rmap_item chain to find back the mapping. This
> would be needed on I/O error for pending writeback of a newly write
> protected page, so one can argue that the overhead of the chain lookup
> to find back the mapping against which to report IO error, is an
> acceptable cost.

Ehm nope. bio_vec is a generic container for pages, requiring
buffer_heads to be able to do IO would be insanity.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Lsf-pc] [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Thu, Feb 01, 2018 at 08:34:58AM -0700, Jens Axboe wrote:
> On 1/31/18 11:13 AM, Jerome Glisse wrote:
> > That's one solution, another one is to have struct bio_vec store
> > buffer_head pointer and not page pointer, from buffer_head you can
> > find struct page and using buffer_head and struct page pointer you
> > can walk the KSM rmap_item chain to find back the mapping. This
> > would be needed on I/O error for pending writeback of a newly write
> > protected page, so one can argue that the overhead of the chain lookup
> > to find back the mapping against which to report IO error, is an
> > acceptable cost.
> 
> Ehm nope. bio_vec is a generic container for pages, requiring
> buffer_heads to be able to do IO would be insanity.

The extra pointer dereference would be killing performance ? Note that
i am not saying have one vec entry per buffer_head but keep thing as
they are and run the following semantic patch:

@@
struct bio_vec *bvec;
expression E;
@@
-bvec->bv_page = E;
+bvec_set_page(bvec, E);

@@
struct bio_vec *bvec;
@@
-bvec->bv_page
+bvec_get_page(bvec);

Then inside struct bio_vec:
s/struct page *bv_head;/struct buffer_head *bv_bh;/

Finally add:
struct page *bvec_get_page(const struct bio_vec *bvec)
{
    return bvec->bv_bh->page;
}

void bvec_set_page(struct bio_vec *bvec, struct page *page)
{
    bvec->bv_bh = first_buffer_head(page);
}

Well you get the idea. Point is that it just add one more pointer
dereference so one more memory lookup. But if it is an issue they
are other way to achieve what i want. For instance i can have a
flags in the address store (1 bit) and make the extra dereference
only needed for write protected page. Or the other solution in
previous email, or something i haven't thought of yet :)

Like i said i don't think i will change the block subsystem, for
block i would only need to change if i ever want to allow write
protection to happen before pending writeback completion. Which
as of now feels to me like a micro-optimization that i might never
need.

In any case i am happy to discuss my ideas and try to find one
that people likes :)

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Lsf-pc] [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jens Axboe]

On 2/1/18 8:57 AM, Jerome Glisse wrote:
> On Thu, Feb 01, 2018 at 08:34:58AM -0700, Jens Axboe wrote:
>> On 1/31/18 11:13 AM, Jerome Glisse wrote:
>>> That's one solution, another one is to have struct bio_vec store
>>> buffer_head pointer and not page pointer, from buffer_head you can
>>> find struct page and using buffer_head and struct page pointer you
>>> can walk the KSM rmap_item chain to find back the mapping. This
>>> would be needed on I/O error for pending writeback of a newly write
>>> protected page, so one can argue that the overhead of the chain lookup
>>> to find back the mapping against which to report IO error, is an
>>> acceptable cost.
>>
>> Ehm nope. bio_vec is a generic container for pages, requiring
>> buffer_heads to be able to do IO would be insanity.
> 
> The extra pointer dereference would be killing performance ?

No, I'm saying that requiring a buffer_head to be able to do IO
is insanity. That's how things used to be in the pre-2001 days.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Lsf-pc] [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Thu, Feb 01, 2018 at 09:00:13AM -0700, Jens Axboe wrote:
> On 2/1/18 8:57 AM, Jerome Glisse wrote:
> > On Thu, Feb 01, 2018 at 08:34:58AM -0700, Jens Axboe wrote:
> >> On 1/31/18 11:13 AM, Jerome Glisse wrote:
> >>> That's one solution, another one is to have struct bio_vec store
> >>> buffer_head pointer and not page pointer, from buffer_head you can
> >>> find struct page and using buffer_head and struct page pointer you
> >>> can walk the KSM rmap_item chain to find back the mapping. This
> >>> would be needed on I/O error for pending writeback of a newly write
> >>> protected page, so one can argue that the overhead of the chain lookup
> >>> to find back the mapping against which to report IO error, is an
> >>> acceptable cost.
> >>
> >> Ehm nope. bio_vec is a generic container for pages, requiring
> >> buffer_heads to be able to do IO would be insanity.
> > 
> > The extra pointer dereference would be killing performance ?
> 
> No, I'm saying that requiring a buffer_head to be able to do IO
> is insanity. That's how things used to be in the pre-2001 days.

Oh ok i didn't thought it would be a problem, iirc it seemed to me that
nobh fs were allocating a buffer_head just do I/O but my memory is probably
confuse. Well i can use the one bit flag idea then allmost same semantic
patch but if flag is (ie page is write protected)set then to get the real
page address you have to do an extra memory dereference. So it would add
an extra test for common existing case and an extra derefence for the write
protect case. No need for buffer_head.

Thanks for pointing out this buffer_head thing :)

Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Kirill A. Shutemov]

On Wed, Jan 31, 2018 at 12:42:45PM -0500, Jerome Glisse wrote:
> The overall idea i have is that in any place in the kernel (except memory reclaim
> but that's ok) we can either get mapping or buffer_head information without relying
> on struct page and if we have either one and a struct page then we can find the
> other one.

Why is it okay for reclaim?

And what about physical memory scanners that doesn't have any side information
about the page they step onto?

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Thu, Feb 01, 2018 at 03:27:52PM +0300, Kirill A. Shutemov wrote:
> On Wed, Jan 31, 2018 at 12:42:45PM -0500, Jerome Glisse wrote:
> > The overall idea i have is that in any place in the kernel (except memory reclaim
> > but that's ok) we can either get mapping or buffer_head information without relying
> > on struct page and if we have either one and a struct page then we can find the
> > other one.
> 
> Why is it okay for reclaim?
> 
> And what about physical memory scanners that doesn't have any side information
> about the page they step onto?

Reclaim is only interested in unmapping and reclaiming, KSM already provide
special function for unmapping (rmap walk) so it is just about extending that
for file back page.

For physical memory scanners it depends on what their individual objectives are.
I have not reviewed them but had the feeling that i could add special KSM helpers
to achieve aims of each one of them.

The mapping information is not lost, it would just not be easily accessible
for those write protected pages.

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Igor Stoppa]

On 30/01/18 02:43, Jerome Glisse wrote:

[...]

> Maybe we can kill page->mapping altogether as a result of this. However this is
> not my motivation at this time.

We had a discussion some time ago

http://www.openwall.com/lists/kernel-hardening/2017/07/07/7

where you advised to use it for tracking pmalloc pages vs area, which
generated this patch:

http://www.openwall.com/lists/kernel-hardening/2018/01/24/7

Could you please comment what wold happen to the shortcut from struct
page to vm_struct that this patch is now introducing?


--
thanks, igor

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [LSF/MM TOPIC] Killing reliance on struct page->mapping

[Jerome Glisse]

On Wed, Jan 31, 2018 at 07:09:48PM +0200, Igor Stoppa wrote:
> On 30/01/18 02:43, Jerome Glisse wrote:
> 
> [...]
> 
> > Maybe we can kill page->mapping altogether as a result of this. However this is
> > not my motivation at this time.
> 
> We had a discussion some time ago
> 
> http://www.openwall.com/lists/kernel-hardening/2017/07/07/7
> 
> where you advised to use it for tracking pmalloc pages vs area, which
> generated this patch:
> 
> http://www.openwall.com/lists/kernel-hardening/2018/01/24/7
> 
> Could you please comment what wold happen to the shortcut from struct
> page to vm_struct that this patch is now introducing?

Sadly struct page fields means different thing depending on the context
in which the page is use. This is confusing i know. So when i say kill
page->mapping i am not saying shrink the struct page and remove that
field, i am saying maybe we can kill current user of page->mapping
for regular process page (ie page that are in some mmap() area of a
process).

Other use of that field in different context like yours are not affected
by this change and can ignore it alltogether.

Hope this clarify it :)

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>