On Thu, Jun 01, 2017 at 12:53:53PM +0300, Nikolay Borisov wrote:
> > +static int ref_get_fields(struct extent_buffer *eb, int slot,
> > + unsigned long ref_ptr, u32 *namelen, char **name,
> > + u64 *index)
> > {
> > struct btrfs_inode_ref *ref;
> >
> > ref = (struct btrfs_inode_ref *)ref_ptr;
> >
> > *namelen = btrfs_inode_ref_name_len(eb, ref);
> > + if (!btrfs_is_namelen_valid(eb, slot, (unsigned long)(ref + 1),
> > + *namelen))
> > + return -EIO;
>
> I'd like to use this to raise a point - shouldn't btrfs actually try to
> utilize a bit more the EUCLEAN error code. Both xfs/ext4 do define their
> EFSCORRUPTED to EUCLEAN and signal that a structure is corrupted and
> needs cleaning. Presumably when we btrfs_is_namelen_valid fails (or
> other validation function) this means the data on=disk is corrupted
> rather than there was an error during I/O which -EIO implies. Currently
> btrfs uses EUCLEAN in only 3 instances in disk-io.c I'd like to solicit
> opinions from other developers what their take on that is?
We've come accross EUCLEAN in the past, eg. in this thread
https://lkml.kernel.org/r/1473870467-18721-1-git-send-email-bo.li.liu@xxxxxxxxxx
Use of EUCLEAN is really sparse in btrfs and now everything is EIO,
while we could make a distinction between EIO and EUCLEAN. It "just"
needs to evaluate all callpaths if the errorcode is expected, the
semantics is defined and the behaviour is consistent with the existing
filesystems that use it.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
