* [PATCH] fs-private data in struct block_device
@ 2004-01-01 16:42 Christoph Hellwig
2004-01-01 23:10 ` Andrew Morton
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2004-01-01 16:42 UTC (permalink / raw)
To: akpm, viro; +Cc: linux-fsdevel
Al had some critism about a per-blockdev XFS datastructure a while ago
and so I tried to follow the suggestion in favour of using struct
block_device directly. Unfortunately XFS requires a little bit more
information than what's available in struct block_device, in particular
whether the blocksize (not sector size) for that devices is identical
to the page size, and we can't really get that from elsewhere due to
layering constraints.
Nathan Scott scott suggested turning the bd_invalidated fields into a
flags field where the filesystem uses the so far unused flags for it's
purposed (similar to struct buffer_head), but I think maybe the variant
below that just adds a unsigned long for filesystem usage might be more
useful as it also allows storing a pointer if needed. Comments?
--- 1.276/include/linux/fs.h Mon Dec 29 22:37:20 2003
+++ edited/include/linux/fs.h Thu Jan 1 13:42:08 2004
@@ -353,6 +353,7 @@
int bd_invalidated;
struct gendisk * bd_disk;
struct list_head bd_list;
+ unsigned long bd_fspriv; /* file-system private data */
};
/*
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 16:42 [PATCH] fs-private data in struct block_device Christoph Hellwig
@ 2004-01-01 23:10 ` Andrew Morton
2004-01-01 23:21 ` viro
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andrew Morton @ 2004-01-01 23:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, linux-fsdevel
Christoph Hellwig <hch@lst.de> wrote:
>
> Al had some critism about a per-blockdev XFS datastructure a while ago
> and so I tried to follow the suggestion in favour of using struct
> block_device directly. Unfortunately XFS requires a little bit more
> information than what's available in struct block_device, in particular
> whether the blocksize (not sector size) for that devices is identical
> to the page size, and we can't really get that from elsewhere due to
> layering constraints.
Why doesn't `if (bdev->bd_block_size == PAGE_SIZE)' work?
Or sb->s_blocksize?
Generally, redundant info like this is a hassle because it has to be kept
coherent with the thing which it is duplicating.
I suspect I don't understand the problem.
> Nathan Scott scott suggested turning the bd_invalidated fields into a
> flags field where the filesystem uses the so far unused flags for it's
> purposed (similar to struct buffer_head), but I think maybe the variant
> below that just adds a unsigned long for filesystem usage might be more
> useful as it also allows storing a pointer if needed. Comments?
>
>
> --- 1.276/include/linux/fs.h Mon Dec 29 22:37:20 2003
> +++ edited/include/linux/fs.h Thu Jan 1 13:42:08 2004
> @@ -353,6 +353,7 @@
> int bd_invalidated;
> struct gendisk * bd_disk;
> struct list_head bd_list;
> + unsigned long bd_fspriv; /* file-system private data */
> };
>
> /*
Seems sane. You'll need to decide whether that field should be operated
upon with atomic ops. If so, document it. If not, document its locking,
please.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:10 ` Andrew Morton
@ 2004-01-01 23:21 ` viro
2004-01-01 23:23 ` Christoph Hellwig
2004-01-01 23:22 ` Christoph Hellwig
2004-01-02 2:43 ` Jeff Garzik
2 siblings, 1 reply; 10+ messages in thread
From: viro @ 2004-01-01 23:21 UTC (permalink / raw)
To: Andrew Morton; +Cc: Christoph Hellwig, linux-fsdevel
On Thu, Jan 01, 2004 at 03:10:17PM -0800, Andrew Morton wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> >
> > Al had some critism about a per-blockdev XFS datastructure a while ago
> > and so I tried to follow the suggestion in favour of using struct
> > block_device directly. Unfortunately XFS requires a little bit more
> > information than what's available in struct block_device, in particular
> > whether the blocksize (not sector size) for that devices is identical
> > to the page size, and we can't really get that from elsewhere due to
> > layering constraints.
>
> Why doesn't `if (bdev->bd_block_size == PAGE_SIZE)' work?
>
> Or sb->s_blocksize?
>
> Generally, redundant info like this is a hassle because it has to be kept
> coherent with the thing which it is duplicating.
>
> I suspect I don't understand the problem.
> > Nathan Scott scott suggested turning the bd_invalidated fields into a
> > flags field where the filesystem uses the so far unused flags for it's
> > purposed (similar to struct buffer_head), but I think maybe the variant
> > below that just adds a unsigned long for filesystem usage might be more
> > useful as it also allows storing a pointer if needed. Comments?
Obvious: we need at least some rules regarding who can and who can not
play with that field. IOW, which filesystem?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:10 ` Andrew Morton
2004-01-01 23:21 ` viro
@ 2004-01-01 23:22 ` Christoph Hellwig
2004-01-02 2:43 ` Jeff Garzik
2 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2004-01-01 23:22 UTC (permalink / raw)
To: Andrew Morton; +Cc: viro, linux-fsdevel
On Thu, Jan 01, 2004 at 03:10:17PM -0800, Andrew Morton wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> >
> > Al had some critism about a per-blockdev XFS datastructure a while ago
> > and so I tried to follow the suggestion in favour of using struct
> > block_device directly. Unfortunately XFS requires a little bit more
> > information than what's available in struct block_device, in particular
> > whether the blocksize (not sector size) for that devices is identical
> > to the page size, and we can't really get that from elsewhere due to
> > layering constraints.
>
> Why doesn't `if (bdev->bd_block_size == PAGE_SIZE)' work?
Because it's different from the XFS block size. In XFS terms it's
the sector size.
> Or sb->s_blocksize?
Would work, except that we don't get at per-superblock data in the
place we need that information - it's in pagebuf which is the IRIX
buffercache interface reimplemented ontop of the Linux pagecache,
and that has neither access to Linux VFS nor XFS-internal data
structures.
> Seems sane. You'll need to decide whether that field should be operated
> upon with atomic ops. If so, document it. If not, document its locking,
> please.
That's completly left to the filesystem. For XFS it would be set once
at mount time and then left constant.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:21 ` viro
@ 2004-01-01 23:23 ` Christoph Hellwig
2004-01-01 23:28 ` viro
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2004-01-01 23:23 UTC (permalink / raw)
To: viro; +Cc: Andrew Morton, linux-fsdevel
On Thu, Jan 01, 2004 at 11:21:37PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> Obvious: we need at least some rules regarding who can and who can not
> play with that field. IOW, which filesystem?
Okay, let's make it more general. Throw the filesystem completly out of
the few and say the private data can be used by the owner of that
blockdevice as established by bd_claim, okay?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:23 ` Christoph Hellwig
@ 2004-01-01 23:28 ` viro
2004-01-01 23:32 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: viro @ 2004-01-01 23:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrew Morton, linux-fsdevel
On Fri, Jan 02, 2004 at 12:23:45AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 01, 2004 at 11:21:37PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > Obvious: we need at least some rules regarding who can and who can not
> > play with that field. IOW, which filesystem?
>
> Okay, let's make it more general. Throw the filesystem completly out of
> the few and say the private data can be used by the owner of that
> blockdevice as established by bd_claim, okay?
OK... One question: what rules do you want wrt multiple claims with the
same owner?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:28 ` viro
@ 2004-01-01 23:32 ` Christoph Hellwig
2004-01-04 22:04 ` Randy.Dunlap
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2004-01-01 23:32 UTC (permalink / raw)
To: viro; +Cc: Andrew Morton, linux-fsdevel
On Thu, Jan 01, 2004 at 11:28:32PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > Okay, let's make it more general. Throw the filesystem completly out of
> > the few and say the private data can be used by the owner of that
> > blockdevice as established by bd_claim, okay?
>
> OK... One question: what rules do you want wrt multiple claims with the
> same owner?
I'd say this is the owners problem and add a big comment about that.
What about something like the version below?:
--- 1.276/include/linux/fs.h Mon Dec 29 22:37:20 2003
+++ edited/include/linux/fs.h Fri Jan 2 01:30:42 2004
@@ -353,6 +353,13 @@
int bd_invalidated;
struct gendisk * bd_disk;
struct list_head bd_list;
+ /*
+ * Private data. You must have bd_claimed the block_device
+ * to use this. NOTE: bd_claims allows an owner to claim
+ * the same device multiple timers, the owner must take special
+ * care to not mess up bd_private for that case.
+ */
+ unsigned long bd_private;
};
/*
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:10 ` Andrew Morton
2004-01-01 23:21 ` viro
2004-01-01 23:22 ` Christoph Hellwig
@ 2004-01-02 2:43 ` Jeff Garzik
2004-01-02 8:48 ` Christoph Hellwig
2 siblings, 1 reply; 10+ messages in thread
From: Jeff Garzik @ 2004-01-02 2:43 UTC (permalink / raw)
To: Andrew Morton; +Cc: Christoph Hellwig, viro, linux-fsdevel
On Thu, Jan 01, 2004 at 03:10:17PM -0800, Andrew Morton wrote:
> > --- 1.276/include/linux/fs.h Mon Dec 29 22:37:20 2003
> > +++ edited/include/linux/fs.h Thu Jan 1 13:42:08 2004
> > @@ -353,6 +353,7 @@
> > int bd_invalidated;
> > struct gendisk * bd_disk;
> > struct list_head bd_list;
> > + unsigned long bd_fspriv; /* file-system private data */
> > };
> >
> > /*
>
> Seems sane. You'll need to decide whether that field should be operated
> upon with atomic ops. If so, document it. If not, document its locking,
> please.
It looks like the standard normally-a-pointer thing that's completely
managed opaquely to the block device layer. In fact, I actually prefer
that these be (void*) unless there is a common use as an integer.
Jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-02 2:43 ` Jeff Garzik
@ 2004-01-02 8:48 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2004-01-02 8:48 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, viro, linux-fsdevel
On Thu, Jan 01, 2004 at 09:43:17PM -0500, Jeff Garzik wrote:
> It looks like the standard normally-a-pointer thing that's completely
> managed opaquely to the block device layer. In fact, I actually prefer
> that these be (void*) unless there is a common use as an integer.
The XFS usage would be as int (actually as boolean)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs-private data in struct block_device
2004-01-01 23:32 ` Christoph Hellwig
@ 2004-01-04 22:04 ` Randy.Dunlap
0 siblings, 0 replies; 10+ messages in thread
From: Randy.Dunlap @ 2004-01-04 22:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, akpm, linux-fsdevel
On Fri, 2 Jan 2004 00:32:27 +0100 Christoph Hellwig <hch@lst.de> wrote:
| On Thu, Jan 01, 2004 at 11:28:32PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
| > > Okay, let's make it more general. Throw the filesystem completly out of
| > > the few and say the private data can be used by the owner of that
| > > blockdevice as established by bd_claim, okay?
| >
| > OK... One question: what rules do you want wrt multiple claims with the
| > same owner?
|
| I'd say this is the owners problem and add a big comment about that.
| What about something like the version below?:
|
|
| --- 1.276/include/linux/fs.h Mon Dec 29 22:37:20 2003
| +++ edited/include/linux/fs.h Fri Jan 2 01:30:42 2004
| @@ -353,6 +353,13 @@
| int bd_invalidated;
| struct gendisk * bd_disk;
| struct list_head bd_list;
| + /*
| + * Private data. You must have bd_claimed the block_device
| + * to use this. NOTE: bd_claims allows an owner to claim
| + * the same device multiple timers, the owner must take special
times;
| + * care to not mess up bd_private for that case.
preferably: not to mess up
| + */
| + unsigned long bd_private;
| };
|
| /*
| -
just nits.
--
~Randy
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2004-01-04 22:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-01-01 16:42 [PATCH] fs-private data in struct block_device Christoph Hellwig
2004-01-01 23:10 ` Andrew Morton
2004-01-01 23:21 ` viro
2004-01-01 23:23 ` Christoph Hellwig
2004-01-01 23:28 ` viro
2004-01-01 23:32 ` Christoph Hellwig
2004-01-04 22:04 ` Randy.Dunlap
2004-01-01 23:22 ` Christoph Hellwig
2004-01-02 2:43 ` Jeff Garzik
2004-01-02 8:48 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox