On 1/24/22 2:05 AM, Jan Kara wrote:

External email: Use caution opening links or attachments


Hello,

On Sun 23-01-22 23:52:07, John Hubbard wrote:

quoted

Background: despite having very little experience in the block and bio
layers, I am attempting to convert the Direct IO parts of them from
using get_user_pages_fast(), to pin_user_pages_fast(). This requires the
use of a corresponding special release call: unpin_user_pages(), instead
of the generic put_page().

Fortunately, Christoph Hellwig has observed [1] (more than once [2]) that
only "a little" refactoring is required, because it is *almost* true
that bio_release_pages() could just be switched over from calling
put_page(), to unpin_user_page(). The "not quite" part is mainly due to
the zero page. There are a few write paths that pad zeroes, and they use
the zero page.

That's where I'd like some advice. How to refactor things, so that the
zero page does not end up in the list of pages that bio_release_pages()
acts upon?

this maybe wrong but thinking out loudly, have you consider adding a 
ZERO_PAGE() address check since it should have a unique same
address for each ZERO_PAGE() (unless I'm totally wrong here) and
using this check you can distinguish between ZERO_PAGE() and
non ZERO_PAGE() on the bio list in bio_release_pages().

quoted

To ground this in reality, one of the partial call stacks is:

do_direct_IO()
     dio_zero_block()
         page = ZERO_PAGE(0); <-- This is a problem

I'm not sure what to use, instead of that zero page! The zero page
doesn't need to be allocated nor tracked, and so any replacement
approaches would need either other storage, or some horrid scheme that I
won't go so far as to write on the screen. :)

Well, I'm not sure if you consider this ugly but currently we use
get_page() in that path exactly so that bio_release_pages() does not have
to care about zero page. So now we could grab pin on the zero page instead
through try_grab_page() or something like that...

       

submit_page_section() does call get_page() in that same path 
irrespective of whether it is ZERO_PAGE() or not, this actually
makes accounting much easier and we also avoid  any special case
for ZERO_PAGE().

dio_zero_block()
  submit_page_section()
    get_page()


That also means that on completion of dio for each bio we also call
put_page() from bio_release_page() path.

                                                           Honza

--
Jan Kara [off-list ref]
SUSE Labs, CR