Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007, Andrew Morton wrote: > Well that's the "locking" protocol then: each instance of this structure is > only ever touched by a single thread, yes? Yes. Each do_revoke() call creates a new instance. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007, Andrew Morton wrote: Well that's the locking protocol then: each instance of this structure is only ever touched by a single thread, yes? Yes. Each do_revoke() call creates a new instance. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007 23:32:28 +0300 (EEST)
Pekka J Enberg <[EMAIL PROTECTED]> wrote:
> On Thu, 3 May 2007, Andrew Morton wrote:
> > > +/**
> > > + * fileset - an array of file pointers.
> > > + * @files:the array of file pointers
> > > + * @nr: number of elements in the array
> > > + * @end: index to next unused file pointer
> > > + */
> > > +struct fileset {
> > > + struct file **files;
> > > + unsigned long nr;
> > > + unsigned long end;
> > > +};
> >
> > What's the locking protocol for all this?
>
> What do you mean? There is no concurrent access going on here.
Well that's the "locking" protocol then: each instance of this structure is
only ever touched by a single thread, yes?
> On Thu, 3 May 2007, Andrew Morton wrote:
> > > +static void free_fset(struct fileset *fset)
> > > +{
> > > + int i;
> > > +
> > > + for (i = fset->end; i < fset->nr; i++)
> > > + fput(fset->files[i]);
> > > +
> > > + kfree(fset->files);
> > > + kfree(fset);
> > > +}
> >
> > Confused. Shouldn't it be
> >
> > for (i = 0; i < fset->end; i++)
>
> No. The fset->end is an index to the first _unused_ file pointer. All
> entries before that are in use by revoked file descriptors so we don't
> want to fput() them.
>
OK.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007, Andrew Morton wrote:
> > +/**
> > + * fileset - an array of file pointers.
> > + * @files:the array of file pointers
> > + * @nr: number of elements in the array
> > + * @end: index to next unused file pointer
> > + */
> > +struct fileset {
> > + struct file **files;
> > + unsigned long nr;
> > + unsigned long end;
> > +};
>
> What's the locking protocol for all this?
What do you mean? There is no concurrent access going on here.
On Thu, 3 May 2007, Andrew Morton wrote:
> > +static void free_fset(struct fileset *fset)
> > +{
> > + int i;
> > +
> > + for (i = fset->end; i < fset->nr; i++)
> > + fput(fset->files[i]);
> > +
> > + kfree(fset->files);
> > + kfree(fset);
> > +}
>
> Confused. Shouldn't it be
>
> for (i = 0; i < fset->end; i++)
No. The fset->end is an index to the first _unused_ file pointer. All
entries before that are in use by revoked file descriptors so we don't
want to fput() them.
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007 17:53:07 +0300 (EEST)
Pekka J Enberg <[EMAIL PROTECTED]> wrote:
> From: Pekka Enberg <[EMAIL PROTECTED]>
>
> The revoke_table struct is overloaded because it serves two purposes:
> it manages the pre-allocated set of files and tracks the revoke
> operation so that we know where to start restore if the operation
> fails. This splits file set management to separate struct fileset and
> renames struct revoke_table to revoke_details.
>
> Signed-off-by: Pekka Enberg <[EMAIL PROTECTED]>
> ---
> fs/revoke.c | 171
> +++-
> 1 file changed, 101 insertions(+), 70 deletions(-)
>
> Index: 26-mm/fs/revoke.c
> ===
> --- 26-mm.orig/fs/revoke.c2007-05-03 17:10:56.0 +0300
> +++ 26-mm/fs/revoke.c 2007-05-03 17:14:49.0 +0300
> @@ -18,19 +18,71 @@ * Copyright (C) 2006-2007 Pekka Enberg
> #include
> #include
>
> -/*
> - * This is used for pre-allocating an array of file pointers so that we don't
> - * have to do memory allocation under tasklist_lock.
> +/**
> + * fileset - an array of file pointers.
> + * @files:the array of file pointers
> + * @nr: number of elements in the array
> + * @end: index to next unused file pointer
> + */
> +struct fileset {
> + struct file **files;
> + unsigned long nr;
> + unsigned long end;
> +};
What's the locking protocol for all this?
> +static void free_fset(struct fileset *fset)
> +{
> + int i;
> +
> + for (i = fset->end; i < fset->nr; i++)
> + fput(fset->files[i]);
> +
> + kfree(fset->files);
> + kfree(fset);
> +}
Confused. Shouldn't it be
for (i = 0; i < fset->end; i++)
?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007 17:53:07 +0300 (EEST)
Pekka J Enberg [EMAIL PROTECTED] wrote:
From: Pekka Enberg [EMAIL PROTECTED]
The revoke_table struct is overloaded because it serves two purposes:
it manages the pre-allocated set of files and tracks the revoke
operation so that we know where to start restore if the operation
fails. This splits file set management to separate struct fileset and
renames struct revoke_table to revoke_details.
Signed-off-by: Pekka Enberg [EMAIL PROTECTED]
---
fs/revoke.c | 171
+++-
1 file changed, 101 insertions(+), 70 deletions(-)
Index: 26-mm/fs/revoke.c
===
--- 26-mm.orig/fs/revoke.c2007-05-03 17:10:56.0 +0300
+++ 26-mm/fs/revoke.c 2007-05-03 17:14:49.0 +0300
@@ -18,19 +18,71 @@ * Copyright (C) 2006-2007 Pekka Enberg
#include linux/revoked_fs_i.h
#include linux/syscalls.h
-/*
- * This is used for pre-allocating an array of file pointers so that we don't
- * have to do memory allocation under tasklist_lock.
+/**
+ * fileset - an array of file pointers.
+ * @files:the array of file pointers
+ * @nr: number of elements in the array
+ * @end: index to next unused file pointer
+ */
+struct fileset {
+ struct file **files;
+ unsigned long nr;
+ unsigned long end;
+};
What's the locking protocol for all this?
+static void free_fset(struct fileset *fset)
+{
+ int i;
+
+ for (i = fset-end; i fset-nr; i++)
+ fput(fset-files[i]);
+
+ kfree(fset-files);
+ kfree(fset);
+}
Confused. Shouldn't it be
for (i = 0; i fset-end; i++)
?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007, Andrew Morton wrote:
+/**
+ * fileset - an array of file pointers.
+ * @files:the array of file pointers
+ * @nr: number of elements in the array
+ * @end: index to next unused file pointer
+ */
+struct fileset {
+ struct file **files;
+ unsigned long nr;
+ unsigned long end;
+};
What's the locking protocol for all this?
What do you mean? There is no concurrent access going on here.
On Thu, 3 May 2007, Andrew Morton wrote:
+static void free_fset(struct fileset *fset)
+{
+ int i;
+
+ for (i = fset-end; i fset-nr; i++)
+ fput(fset-files[i]);
+
+ kfree(fset-files);
+ kfree(fset);
+}
Confused. Shouldn't it be
for (i = 0; i fset-end; i++)
No. The fset-end is an index to the first _unused_ file pointer. All
entries before that are in use by revoked file descriptors so we don't
want to fput() them.
Pekka
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] revoke: change revoke_table to fileset and revoke_details
On Thu, 3 May 2007 23:32:28 +0300 (EEST)
Pekka J Enberg [EMAIL PROTECTED] wrote:
On Thu, 3 May 2007, Andrew Morton wrote:
+/**
+ * fileset - an array of file pointers.
+ * @files:the array of file pointers
+ * @nr: number of elements in the array
+ * @end: index to next unused file pointer
+ */
+struct fileset {
+ struct file **files;
+ unsigned long nr;
+ unsigned long end;
+};
What's the locking protocol for all this?
What do you mean? There is no concurrent access going on here.
Well that's the locking protocol then: each instance of this structure is
only ever touched by a single thread, yes?
On Thu, 3 May 2007, Andrew Morton wrote:
+static void free_fset(struct fileset *fset)
+{
+ int i;
+
+ for (i = fset-end; i fset-nr; i++)
+ fput(fset-files[i]);
+
+ kfree(fset-files);
+ kfree(fset);
+}
Confused. Shouldn't it be
for (i = 0; i fset-end; i++)
No. The fset-end is an index to the first _unused_ file pointer. All
entries before that are in use by revoked file descriptors so we don't
want to fput() them.
OK.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
