Re: [RFC v2 0/5] remove page_endio()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> Open questions:
>> - Willy pointed out that the calls to folio_set_error() and
>>   folio_clear_uptodate() are not needed anymore in the read path when an
>>   error happens[2]. I still don't understand 100% why they aren't needed
>>   anymore as I see those functions are still called in iomap. It will be
>>   good to put that rationale as a part of the commit message.
> 
> page_endio() was generic.  It needed to handle a lot of cases.  When it's
> being inlined into various completion routines, we know which cases we
> need to handle and can omit all the cases which we don't.
> 
> We know the uptodate flag is clear.  If the uptodate flag is set,
> we don't call the filesystem's read path.  Since we know it's clear,
> we don't need to clear it.
> 
Got it.

> We don't need to set the error flag.  Only some filesystems still use
> the error flag, and orangefs isn't one of them.  I'd like to get rid
> of the error flag altogether, and I've sent patches in the past which
> get us a lot closer to that desired outcome.  Not sure we're there yet.
> Regardless, generic code doesn't check the error flag.

Thanks for the explanation. I think found the series you are referring here.

https://lore.kernel.org/linux-mm/20220527155036.524743-1-willy@xxxxxxxxxxxxx/#t

I see orangefs is still setting the error flag in orangefs_read_folio(), so
it should be removed at some point?

I also changed mpage to **not set** the error flag in the read path. It does beg
the question whether block_read_full_folio() and iomap_finish_folio_read() should
also follow the suit.

--
Pankaj



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux