On Tuesday, May 28, 2013 10:49:31 Abhijith Das wrote:
> Zack, thanks for taking a peek at the patches.
>
> > Some quick things that struck me as I glanced through the patches:
> >
> > - Please post the patch series, don't make us go digging through
> >
> > bugzilla.
>
> Duly noted. I wasn't hoping for my patches to be a serious submission, just
> something I wrote up as a POC. I was mainly looking to (re)start a
> conversation about readdirplus to see what's the best way to go about doing
> this. Your point applies nonetheless; I'll post my patchset again,
> properly.
Speaking for the nfs-ganesha project, one of the issues that we couldn't sort
out at lsf was whether to to include xattrs. As far as ganesha is concerned,
the xstat struct is sufficient. At readdir time, we pretty much just want to
build our cache entries and get basic stat info. The only case where we'd
really need xattrs would be for acls but that is usually later in the protocol
op sequence. I'd just like to get the path from readdir+ thru the callbacks
to the xdr into the reply as simple as possible. That and get enough in the
cache entry to be ready for the next step.
> > - Don't use variable size types in the ABI or you'll have to add compat_
> >
> > wrappers to fix it all up on the stack when going between 32bit
> > userspace and 64bit kernelspace. This is going to be especially nasty
> > if this is a giant sequence of variable length blobs.
> >
> > +struct linux_xdirent {
> > + unsigned long xd_ino;
> > + char xd_type;
> > + unsigned long xd_off;
> > + struct xstat xd_stat;
> > + unsigned long xd_reclen;
> > + struct xdirent_blob xd_blob;
> > +};
> >
> > Notice how, in contrast, David was careful to use naturally aligned
> > fixed-width types in his xstat patch.
>
> Yes, you're right. I'll fix this.
>
> > - z
>
> Cheers!
> --Abhi
--
Jim Lieb
Linux Systems Engineer
Panasas Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
