Skip to contentSkip to conversation

get_user_pages() pins in file mappings

LSF/MM TOPIC Jan Kara 2 participants 5 messages Jan 24, 2019 – Feb 6, 2019

Subsystems (2)
lore
Share & tools
lore

Jan Kara

author reviewer
#0 lore raw

This is a joint proposal with Dan Williams, John Hubbard, and Jérôme Glisse.

Last year we've talked with Dan about issues we have with filesystems and GUP [1]. The crux of the problem lies in the fact that there is no coordination (or even awareness) between filesystem working on a page (such as doing writeback) and GUP user modifying page contents and setting it dirty. This can (and we have user reports of this) lead to data corruption, kernel crashes, and other fun.

Since last year we have worked together on solving these problems and we have explored couple dead ends as well as hopefully found solutions to some of the partial problems. So I'd like to give some overview of where we stand and what remains to be solved and get thoughts from wider community about proposed solutions / problems to be solved.

In particular we hope to have reasonably robust mechanism of identifying pages pinned by GUP (patches will be posted soon) - I'd like to run that by MM folks (unless discussion happens on mailing lists before LSF/MM). We also have ideas how filesystems should react to pinned page in their writepages methods - there will be some changes needed in some filesystems to bounce the page if they need stable page contents. So I'd like to explain why we chose to do bouncing to fs people (i.e., why we cannot just wait, skip the page, do something else etc.) to save us from the same discussion with each fs separately and also hash out what the API for filesystems to do this should look like. Finally we plan to keep pinned page permanently dirty - again something I'd like to explain why we do this and gather input from other people.

This should be ideally shared MM + FS session.

[1] https://lwn.net/Articles/753027/

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
#1 lore raw
Quoted 29 lines · Jan Kara

This is a joint proposal with Dan Williams, John Hubbard, and Jérôme Glisse.

Last year we've talked with Dan about issues we have with filesystems and GUP [1]. The crux of the problem lies in the fact that there is no coordination (or even awareness) between filesystem working on a page (such as doing writeback) and GUP user modifying page contents and setting it dirty. This can (and we have user reports of this) lead to data corruption, kernel crashes, and other fun.

Since last year we have worked together on solving these problems and we have explored couple dead ends as well as hopefully found solutions to some of the partial problems. So I'd like to give some overview of where we stand and what remains to be solved and get thoughts from wider community about proposed solutions / problems to be solved.

In particular we hope to have reasonably robust mechanism of identifying pages pinned by GUP (patches will be posted soon) - I'd like to run that by MM folks (unless discussion happens on mailing lists before LSF/MM). We also have ideas how filesystems should react to pinned page in their writepages methods - there will be some changes needed in some filesystems to bounce the page if they need stable page contents. So I'd like to explain why we chose to do bouncing to fs people (i.e., why we cannot just wait, skip the page, do something else etc.) to save us from the same discussion with each fs separately and also hash out what the API for filesystems to do this should look like. Finally we plan to keep pinned page permanently dirty - again something I'd like to explain why we do this and gather input from other people.

Quoted 4 lines

This should be ideally shared MM + FS session.

[1] https://lwn.net/Articles/753027/

Yes! I'd like to attend and discuss this, for sure.

Meanwhile, as usual, I'm a bit late on posting an updated RFC for the page identification part, but that's coming very soon.

thanks,

Signature · 2 lines
John Hubbard
NVIDIA
#2 lore raw
In reply to Jan Kara
Quoted 4 lines · Jan Kara

In particular we hope to have reasonably robust mechanism of identifying pages pinned by GUP (patches will be posted soon) - I'd like to run that by MM folks (unless discussion happens on mailing lists before LSF/MM). We also have ideas how filesystems should react to pinned page in their

Quoted 8 lines

writepages methods - there will be some changes needed in some filesystems to bounce the page if they need stable page contents. So I'd like to explain why we chose to do bouncing to fs people (i.e., why we cannot just wait, skip the page, do something else etc.) to save us from the same discussion with each fs separately and also hash out what the API for filesystems to do this should look like. Finally we plan to keep pinned page permanently dirty - again something I'd like to explain why we do this and gather input from other people.

Hi Jan,

Say, I was just talking through this point with someone on our driver team, and suddenly realized that I'm now slightly confused on one point. If we end up keeping the gup-pinned pages effectively permanently dirty while pinned, then maybe the call sites no longer need to specify "dirty" (or not) when they call put_user_page*()?

In other words, the RFC [1] has this API:

    void put_user_page(struct page *page);
    void put_user_pages_dirty(struct page **pages, unsigned long npages);
    void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
    void put_user_pages(struct page **pages, unsigned long npages);

But maybe we only really need this:

    void put_user_page(struct page *page);
    void put_user_pages(struct page **pages, unsigned long npages);

?

[1] https://lkml.kernel.org/r/[email protected]

thanks,

Signature · 2 lines
John Hubbard
NVIDIA
author reviewer
#3 lore raw

Hi John,

Quoted 30 lines · John Hubbard
Quoted 4 lines · Jan Kara

In particular we hope to have reasonably robust mechanism of identifying pages pinned by GUP (patches will be posted soon) - I'd like to run that by MM folks (unless discussion happens on mailing lists before LSF/MM). We also have ideas how filesystems should react to pinned page in their

Quoted 8 lines

writepages methods - there will be some changes needed in some filesystems to bounce the page if they need stable page contents. So I'd like to explain why we chose to do bouncing to fs people (i.e., why we cannot just wait, skip the page, do something else etc.) to save us from the same discussion with each fs separately and also hash out what the API for filesystems to do this should look like. Finally we plan to keep pinned page permanently dirty - again something I'd like to explain why we do this and gather input from other people.

Hi Jan,

Say, I was just talking through this point with someone on our driver team, and suddenly realized that I'm now slightly confused on one point. If we end up keeping the gup-pinned pages effectively permanently dirty while pinned, then maybe the call sites no longer need to specify "dirty" (or not) when they call put_user_page*()?

In other words, the RFC [1] has this API:

    void put_user_page(struct page *page);
    void put_user_pages_dirty(struct page **pages, unsigned long npages);
    void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
    void put_user_pages(struct page **pages, unsigned long npages);
Quoted 8 lines

But maybe we only really need this:

    void put_user_page(struct page *page);
    void put_user_pages(struct page **pages, unsigned long npages);

?

[1] https://lkml.kernel.org/r/[email protected]

So you are right that if we keep gup-pinned pages dirty, drivers could get away without marking them as such. However I view "keep pages dirty" as an implementation detail, rather than a promise of the API. So I'd like to leave us the flexibility of choosing a different implementation in the future. And as such I'd just leave the put_user_pages_dirty() variants in place.

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
#4 lore raw
Quoted 42 lines · Jan Kara

Hi John,

Quoted 30 lines · John Hubbard
Quoted 4 lines · Jan Kara

In particular we hope to have reasonably robust mechanism of identifying pages pinned by GUP (patches will be posted soon) - I'd like to run that by MM folks (unless discussion happens on mailing lists before LSF/MM). We also have ideas how filesystems should react to pinned page in their

Quoted 8 lines

writepages methods - there will be some changes needed in some filesystems to bounce the page if they need stable page contents. So I'd like to explain why we chose to do bouncing to fs people (i.e., why we cannot just wait, skip the page, do something else etc.) to save us from the same discussion with each fs separately and also hash out what the API for filesystems to do this should look like. Finally we plan to keep pinned page permanently dirty - again something I'd like to explain why we do this and gather input from other people.

Hi Jan,

Say, I was just talking through this point with someone on our driver team, and suddenly realized that I'm now slightly confused on one point. If we end up keeping the gup-pinned pages effectively permanently dirty while pinned, then maybe the call sites no longer need to specify "dirty" (or not) when they call put_user_page*()?

In other words, the RFC [1] has this API:

    void put_user_page(struct page *page);
    void put_user_pages_dirty(struct page **pages, unsigned long npages);
    void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
    void put_user_pages(struct page **pages, unsigned long npages);
Quoted 8 lines

But maybe we only really need this:

    void put_user_page(struct page *page);
    void put_user_pages(struct page **pages, unsigned long npages);

?

[1] https://lkml.kernel.org/r/[email protected]

Quoted 8 lines

So you are right that if we keep gup-pinned pages dirty, drivers could get away without marking them as such. However I view "keep pages dirty" as an implementation detail, rather than a promise of the API. So I'd like to leave us the flexibility of choosing a different implementation in the future. And as such I'd just leave the put_user_pages_dirty() variants in place.

Honza

OK, sounds good. And after all, removing that information from the call sites is easy, but adding it back in would be hell, so leaving it in definitely sounds better. I'm just looking for anything that says "this API isn't ready yet", but I guess we're still OK there.

thanks,

Signature · 2 lines
John Hubbard
NVIDIA