Killing reliance on struct page->mapping

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,
J�r�me

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

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).

n other

like a

M (i am

l the

only on

of vfs

g call

argument

er...

What matter is that the place that call an address_space_operations callback already has mapping =3D=3D page->mapping in many places this is obvious. For instance page just have been looked up using mapping and thus you must have mapping =3D=3D 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 =3D=3D file_inode(file) but i=
might
count in _some_ place on vma->file->f_mapping =3D=3D page->mapping of any n= on 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 r= eclaim
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,
J=E9r=F4me
_______________________________________________
Lsf-pc mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/lsf-pc

Details, please...
_______________________________________________
Lsf-pc mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/lsf-pc

one of

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,
J=E9r=F4me
_______________________________________________
Lsf-pc mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/lsf-pc

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

Jens Axboe

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,
J�r�me

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

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 :)

J�r�me

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
_______________________________________________
Lsf-pc mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/lsf-pc

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,
J�r�me

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

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

[...]

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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,
J�r�me

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