Hi Sam,

> This of course doesn't map well to distributed file systems.  How are
> you going to deal with cached files once the immutable attribute is
> removed?

Hmm.. yeah that is a problem..
I realize that RobR is against adding state at the servers and I think
he has perfectly valid reasons for that but it will be quite easy to
do the cache invalidation if we
- kept that state (of who all opened an immutable file) as a hint
(even if servers crash and restart, clients can detect that and have
them bubble up this information to kmod and have it wipe the cache
clean)
- we allow client-core or kmod to have a listening socket for such messages.

In any case, detecting the case of an immutable bit being removed from
some node in the cluster for subsequent opens of the file is trivial..
We only need to worry about the case
of files that have already been opened. Without the invalidation based
approach, there is no hope for solving this problem then :(

>
> I've attached a patch that adds the ioctls to enable setting the
> immutable with the chattr command.

Awesome! THanks for doing this! A few comments.

- suggest you replace sizeof(uint64_t) with sizeof(val)
-  pvfs2_xattr_get_default and pvfs2_xattr_set_default return either 0
or -ve number. They don't return > 0. so the check can else if (ret ==
0)
- This looks incorrect:

 if(ret >= 0)
+        {
+            return put_user(val, (int __user *)arg);
+        }

Flags:
PVFS_IMMUTABLE_FL != FS_IMMUTABLE_FL
PVFS_APPEND_FL != FS_APPEND_FL
PVFS_NOATIME_FL != FS_NOATIME_FL

Before the put_user(), you should convert val from a PVFS_*_FL to a
FS_*_FL flag I think.
ELse the chattr utility won't understand these flags..
- if(arg & FS_APPEND_FL)
+        {
+            val |= PVFS_IMMUTABLE_FL; <--- PVFS_APPEND_FL
+        }

- should XATTR_CREATE simply be 0?
XATTR_CREATE will fail if a similar xattr already exists I think.
- I think you need to set ret = 0 if(val == 0) in the FS_IOC_SETFLAGS path.
- Btw: Did you test this on x86_64 with 32 bit user-space? DO we need
to implement FS_IOC32_SETFLAGS
or FS_IOC32_GETFLAGS?

thanks,
Murali

>
> -sam
>
>
>
>
> > thanks,
> > Murali
> >
> >> Maybe I'm missing something but I think deleting the file should be
> >> allowed, in fact it should be the only way to remove the immutable
> >> attribute.
> >>
> >> -sam
> >>
> >>> thanks,
> >>> Murali
> >>>>
> >>>> -sam
> >>>>
> >>>> On Aug 17, 2007, at 10:09 PM, Murali Vilayannur wrote:
> >>>>
> >>>>> Sam,
> >>>>> The problem is not in the system call (fsetxattr) but the
> >>>>> arguments
> >>>>> to it..
> >>>>> user.pvfs2.meta_hint is the key and val is actually a uint64
> >>>>> which is
> >>>>> a bitwise OR
> >>>>> of PVFS_IMMUTABLE_FL, other pvfs flags.
> >>>>> modify_val() in pvfs2-xattr.c will give an example of this usage.
> >>>>> Sorry, it is a little convoluted ..:(
> >>>>> but I couldn't/didn't want to do more string parsing on server
> >>>>> side.
> >>>>> Feel free to change that if you think it is needlessly convoluted.
> >>>>> thanks,
> >>>>> Murali
> >>>>>
> >>>>> PS: let me know how the caching patches work out :)
> >>>>> I havent had too much time to play with it since Feb though.
> >>>>> Hope it works :)
> >>>>>
> >>>>>
> >>>>> On 8/17/07, Sam Lang <[EMAIL PROTECTED]> wrote:
> >>>>>>
> >>>>>> Hi Murali,
> >>>>>>
> >>>>>> I wrote a little program to test the performance of the read-
> >>>>>> caching
> >>>>>> immutable file stuff.  With the  attached program, I get a EINVAL
> >>>>>> error on the read of the file after the immutable attribute has
> >>>>>> been
> >>>>>> set (using fsetxattr).  Also, ls -la gives me really strange
> >>>>>> results
> >>>>>> for the files that I've set that immutable attribute on.  In the
> >>>>>> below listing, tmpfile1 and tmpfile10 didn't have the immutable
> >>>>>> attribute set.  It looks like the problem is with the fsetxattr
> >>>>>> system call.  The setfattr util does the same thing.  When I set
> >>>>>> the
> >>>>>> xattr with pvfs2-xattr though, I don't see the corruption in
> >>>>>> listing
> >>>>>> the file.  I'll try to investigate what fsetxattr is doing,
> >>>>>> but are
> >>>>>> you aware of any problems with using the system call?
> >>>>>>
> >>>>>> -sam
> >>>>>>
> >>>>>> [EMAIL PROTECTED]:/tmp/pvfsmnt# ls -la
> >>>>>> total 10260
> >>>>>> drwxrwxrwt 1 slang mpi      4096 2007-08-17 16:35 .
> >>>>>> drwxrwxrwt 5 root  root     4096 2007-08-17 15:47 ..
> >>>>>> drwxrwxrwx 1 slang mpi      4096 2007-08-17 15:47 lost+found
> >>>>>> -rw-r--r-- 1 root  root        0 2007-08-17 16:24 tmpfile1
> >>>>>> -rw-r--r-- 1 root  root 10485760 2007-08-17 16:34 tmpfile10
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile11
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile2
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile3
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile4
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile5
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile6
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile7
> >>>>>> ?--------- ? ?     ?           ?                ? tmpfile9
> >>>>>> [EMAIL PROTECTED]:/tmp/pvfsmnt#
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Feb 20, 2007, at 1:06 AM, Murali Vilayannur wrote:
> >>>>>>
> >>>>>>> Hi all,
> >>>>>>> Finally, I got some time to whip up the read-caching patches for
> >>>>>>> non-mutable files into a semblance of shape and stability.
> >>>>>>> With this patch, I am able to get I/Os to a file (marked
> >>>>>>> immutable)
> >>>>>>> serviced from the page-cache. One can tag a file as immutable by
> >>>>>>> running,
> >>>>>>> ./src/apps/admin/pvfs2-xattr -s -k user.pvfs2.meta_hint -v
> >>>>>>> "+immutable" /path/to/pvfs2-file
> >>>>>>> To verify if a file is indeed tagged immutable,
> >>>>>>> ./src/apps/admin/pvfs2-xattr -t -k user.pvfs2.meta_hint /path/
> >>>>>>> to/
> >>>>>>> pvfs2-file
> >>>>>>> (or)
> >>>>>>> ./src/apps/admin/pvfs2-stat /path/to/pvfs2/file
> >>>>>>>
> >>>>>>> I have also added some preliminary statistics exported via
> >>>>>>> /proc/sys/pvfs2/stats/
> >>>>>>> that can be used as a placeholder for more interesting
> >>>>>>> statistics
> >>>>>>> later on.
> >>>>>>> Currently, it only shows # of reads, writes, hits in thepage-
> >>>>>>> cache
> >>>>>>> and misses.
> >>>>>>>
> >>>>>>> For some reason now, cache hits do not happen across a file
> >>>>>>> close.
> >>>>>>> Within a file open-close session, all reads get serviced from
> >>>>>>> the
> >>>>>>> cache though. Very weird.
> >>>>>>> My hunch is that file pages are somehow getting removed from the
> >>>>>>> radix
> >>>>>>> tree of the address space due to some page-ref counting
> >>>>>>> issues. I
> >>>>>>> will
> >>>>>>> dig into this later this week.
> >>>>>>>
> >>>>>>> In any case, this code should not cause any regression of older
> >>>>>>> code
> >>>>>>> paths (hopefully!) and should not impose any performance
> >>>>>>> penalties for
> >>>>>>> workloads making use of the page-cache because of the way we
> >>>>>>> aggregate
> >>>>>>> cache miss I/Os to the server.
> >>>>>>> It was really nice to be able to make use of the iox()
> >>>>>>> infrastructure
> >>>>>>> that was already in place to service non-contigous file and
> >>>>>>> memory
> >>>>>>> I/O.
> >>>>>>> More details of the implementation is described in the thread
> >>>>>>> below.
> >>>>>>> http://www.beowulf-underground.org/pipermail/pvfs2-developers/
> >>>>>>> 2006-
> >>>>>>> November/002847.html
> >>>>>>> Hopefully, I have addressed most of Pete's comments.
> >>>>>>> More comments and testing welcome!
> >>>>>>> thanks,
> >>>>>>> Murali
> >>>>>>> <read-cache-5.patch>
> >>>>>>> _______________________________________________
> >>>>>>> Pvfs2-developers mailing list
> >>>>>>> Pvfs2-developers@beowulf-underground.org
> >>>>>>> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-
> >>>>>>> developers
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >
>
>
>
>
_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to