On Tue, Apr 05, 2022 at 06:22:48PM -0700, Dan Williams wrote: > On Tue, Apr 5, 2022 at 5:55 PM Jane Chu <jane.chu@xxxxxxxxxx> wrote: > > > > On 3/30/2022 9:18 AM, Darrick J. Wong wrote: > > > On Wed, Mar 30, 2022 at 08:49:29AM -0700, Christoph Hellwig wrote: > > >> On Wed, Mar 30, 2022 at 06:58:21PM +0800, Shiyang Ruan wrote: > > >>> As the code I pasted before, pmem driver will subtract its ->data_offset, > > >>> which is byte-based. And the filesystem who implements ->notify_failure() > > >>> will calculate the offset in unit of byte again. > > >>> > > >>> So, leave its function signature byte-based, to avoid repeated conversions. > > >> > > >> I'm actually fine either way, so I'll wait for Dan to comment. > > > > > > FWIW I'd convinced myself that the reason for using byte units is to > > > make it possible to reduce the pmem failure blast radius to subpage > > > units... but then I've also been distracted for months. :/ > > > > > > > Yes, thanks Darrick! I recall that. > > Maybe just add a comment about why byte unit is used? > > I think we start with page failure notification and then figure out > how to get finer grained through the dax interface in follow-on > changes. Otherwise, for finer grained error handling support, > memory_failure() would also need to be converted to stop upcasting > cache-line granularity to page granularity failures. The native MCE > notification communicates a 'struct mce' that can be in terms of > sub-page bytes, but the memory management implications are all page > based. I assume the FS implications are all FS-block-size based? I wouldn't necessarily make that assumption -- for regular files, the user program is in a better position to figure out how to reset the file contents. For fs metadata, it really depends. In principle, if (say) we could get byte granularity poison info, we could look up the space usage within the block to decide if the poisoned part was actually free space, in which case we can correct the problem by (re)zeroing the affected bytes to clear the poison. Obviously, if the blast radius hits the internal space info or something that was storing useful data, then you'd have to rebuild the whole block (or the whole data structure), but that's not necessarily a given. --D
