Re: [PATCH 1/2] BDI: Provide backing device capability information [try #3]
Andrew Morton <[EMAIL PROTECTED]> wrote: > > The attached patch replaces backing_dev_info::memory_backed with > > capabilitied bitmap. > > Looks sane to me, thanks. > > I hope you got all the conversions correct - breakage in the writeback > dirty accounting manifests in subtle ways. I'll double-check it. I think I got the logic as-was. This is quite easy to check just by looking at the patch. However, the as-was logic should possibly be changed to reflect the fact that the one control that there was has now been split into two. David - 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 1/2] BDI: Provide backing device capability information [try #3]
Andrew Morton [EMAIL PROTECTED] wrote: The attached patch replaces backing_dev_info::memory_backed with capabilitied bitmap. Looks sane to me, thanks. I hope you got all the conversions correct - breakage in the writeback dirty accounting manifests in subtle ways. I'll double-check it. I think I got the logic as-was. This is quite easy to check just by looking at the patch. However, the as-was logic should possibly be changed to reflect the fact that the one control that there was has now been split into two. David - 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 1/2] BDI: Provide backing device capability information [try #3]
David Howells <[EMAIL PROTECTED]> wrote: > > The attached patch replaces backing_dev_info::memory_backed with capabilitied > bitmap. Looks sane to me, thanks. I hope you got all the conversions correct - breakage in the writeback dirty accounting manifests in subtle ways. I'll double-check it. - 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/
[PATCH 1/2] BDI: Provide backing device capability information [try #3]
The attached patch replaces backing_dev_info::memory_backed with capabilitied
bitmap. The capabilities available include:
(*) BDI_CAP_NO_ACCT_DIRTY
Set if the pages associated with this backing device should not be
tracked by the dirty page accounting.
(*) BDI_CAP_NO_WRITEBACK
Set if dirty pages associated with this backing device should not have
writepage() or writepages() invoked upon them to clean them.
(*) Capability markers that indicate what a backing device is capable of
with regard to memory mapping facilities. These flags indicate whether a
device can be mapped directly, whether it can be copied for a mapping,
and whether direct mappings can be read, written and/or executed. This
information is primarily aimed at improving no-MMU private mapping
support.
The patch also provides convenience functions for determining the dirty-page
capabilities available on backing devices directly or on the backing devices
associated with a mapping. These are provided to keep line length down when
checking for the capabilities.
Signed-Off-By: David Howells <[EMAIL PROTECTED]>
---
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-08
11:03:45.0 +
@@ -25,13 +25,39 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned int capabilities; /* Device capabilities */
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - The first two flags control whether dirty pages will contribute to the
+ * VM's accounting and whether writepages() should be called for dirty pages
+ * (something that would not, for example, be appropriate for ramfs)
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_NO_ACCT_DIRTY 0x0001 /* Dirty pages shouldn't
contribute to accounting */
+#define BDI_CAP_NO_WRITEBACK 0x0002 /* Don't write pages back */
+#define BDI_CAP_MAP_COPY 0x0004 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0008 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP 0x0010 /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP 0x0020 /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP 0x0040 /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if defined(VM_MAYREAD) && \
+ (BDI_CAP_READ_MAP != VM_MAYREAD || \
+BDI_CAP_WRITE_MAP != VM_MAYWRITE || \
+BDI_CAP_EXEC_MAP != VM_MAYEXEC)
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
@@ -62,4 +88,17 @@ static inline int bdi_rw_congested(struc
(1 << BDI_write_congested));
}
+#define bdi_cap_writeback_dirty(bdi) \
+ (!((bdi)->capabilities & BDI_CAP_NO_WRITEBACK))
+
+#define bdi_cap_account_dirty(bdi) \
+ (!((bdi)->capabilities & BDI_CAP_NO_ACCT_DIRTY))
+
+#define mapping_cap_writeback_dirty(mapping) \
+ bdi_cap_writeback_dirty((mapping)->backing_dev_info)
+
+#define mapping_cap_account_dirty(mapping) \
+ bdi_cap_account_dirty((mapping)->backing_dev_info)
+
+
#endif /* _LINUX_BACKING_DEV_H */
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c 2005-02-14
12:18:23.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c 2005-03-08
11:09:54.0 +
@@ -238,7 +238,7 @@ void blk_queue_make_request(request_queu
q->make_request_fn = mfn;
q->backing_dev_info.ra_pages = (VM_MAX_READAHEAD * 1024) /
PAGE_CACHE_SIZE;
q->backing_dev_info.state = 0;
- q->backing_dev_info.memory_backed = 0;
+ q->backing_dev_info.capabilities = BDI_CAP_MAP_COPY;
blk_queue_max_sectors(q, MAX_SECTORS);
blk_queue_hardsect_size(q, 512);
blk_queue_dma_alignment(q, 511);
diff -uNrp
[PATCH 1/2] BDI: Provide backing device capability information [try #3]
The attached patch replaces backing_dev_info::memory_backed with capabilitied
bitmap. The capabilities available include:
(*) BDI_CAP_NO_ACCT_DIRTY
Set if the pages associated with this backing device should not be
tracked by the dirty page accounting.
(*) BDI_CAP_NO_WRITEBACK
Set if dirty pages associated with this backing device should not have
writepage() or writepages() invoked upon them to clean them.
(*) Capability markers that indicate what a backing device is capable of
with regard to memory mapping facilities. These flags indicate whether a
device can be mapped directly, whether it can be copied for a mapping,
and whether direct mappings can be read, written and/or executed. This
information is primarily aimed at improving no-MMU private mapping
support.
The patch also provides convenience functions for determining the dirty-page
capabilities available on backing devices directly or on the backing devices
associated with a mapping. These are provided to keep line length down when
checking for the capabilities.
Signed-Off-By: David Howells [EMAIL PROTECTED]
---
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-08
11:03:45.0 +
@@ -25,13 +25,39 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned int capabilities; /* Device capabilities */
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - The first two flags control whether dirty pages will contribute to the
+ * VM's accounting and whether writepages() should be called for dirty pages
+ * (something that would not, for example, be appropriate for ramfs)
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_NO_ACCT_DIRTY 0x0001 /* Dirty pages shouldn't
contribute to accounting */
+#define BDI_CAP_NO_WRITEBACK 0x0002 /* Don't write pages back */
+#define BDI_CAP_MAP_COPY 0x0004 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0008 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP 0x0010 /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP 0x0020 /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP 0x0040 /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if defined(VM_MAYREAD) \
+ (BDI_CAP_READ_MAP != VM_MAYREAD || \
+BDI_CAP_WRITE_MAP != VM_MAYWRITE || \
+BDI_CAP_EXEC_MAP != VM_MAYEXEC)
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
@@ -62,4 +88,17 @@ static inline int bdi_rw_congested(struc
(1 BDI_write_congested));
}
+#define bdi_cap_writeback_dirty(bdi) \
+ (!((bdi)-capabilities BDI_CAP_NO_WRITEBACK))
+
+#define bdi_cap_account_dirty(bdi) \
+ (!((bdi)-capabilities BDI_CAP_NO_ACCT_DIRTY))
+
+#define mapping_cap_writeback_dirty(mapping) \
+ bdi_cap_writeback_dirty((mapping)-backing_dev_info)
+
+#define mapping_cap_account_dirty(mapping) \
+ bdi_cap_account_dirty((mapping)-backing_dev_info)
+
+
#endif /* _LINUX_BACKING_DEV_H */
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c 2005-02-14
12:18:23.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c 2005-03-08
11:09:54.0 +
@@ -238,7 +238,7 @@ void blk_queue_make_request(request_queu
q-make_request_fn = mfn;
q-backing_dev_info.ra_pages = (VM_MAX_READAHEAD * 1024) /
PAGE_CACHE_SIZE;
q-backing_dev_info.state = 0;
- q-backing_dev_info.memory_backed = 0;
+ q-backing_dev_info.capabilities = BDI_CAP_MAP_COPY;
blk_queue_max_sectors(q, MAX_SECTORS);
blk_queue_hardsect_size(q, 512);
blk_queue_dma_alignment(q, 511);
diff -uNrp
Re: [PATCH 1/2] BDI: Provide backing device capability information [try #3]
David Howells [EMAIL PROTECTED] wrote: The attached patch replaces backing_dev_info::memory_backed with capabilitied bitmap. Looks sane to me, thanks. I hope you got all the conversions correct - breakage in the writeback dirty accounting manifests in subtle ways. I'll double-check it. - 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 1/2] BDI: Provide backing device capability information
Miklos Szeredi <[EMAIL PROTECTED]> wrote: > > > Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and > > although it was in conjunction with turning the concepts into bitfields, it > > still stands here. > > OK, obviously Andrew has the final word in this. I just suggested > that it might be safer to have the logic flipped back. > Experience indicates that it's safer to ignore anything I say on this topic. Yeah, it would be better to not flip the logic. - 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 1/2] BDI: Provide backing device capability information
> Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and > although it was in conjunction with turning the concepts into bitfields, it > still stands here. OK, obviously Andrew has the final word in this. I just suggested that it might be safer to have the logic flipped back. Miklos - 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 1/2] BDI: Provide backing device capability information
Miklos Szeredi <[EMAIL PROTECTED]> wrote:
> > It shouldn't silently break... It will refuse to compile. I renamed
> > "memory_backed" to "capabilities".
>
> This will silently break:
>
> static struct backing_dev_info my_bdi = {
>.ra_pages = MY_RA,
>.unplug_io_fn = default_unplug_io_fn,
> };
Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and
although it was in conjunction with turning the concepts into bitfields, it
still stands here.
David
-
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 1/2] BDI: Provide backing device capability information
> > Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY
> > and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way
> > out of tree filesystems that implicitly zero bdi->memory_backed
> > wouldn't _silently_ break. E.g. fuse does this, though it would not
> > actually break since it doesn't dirty any pages currently. I have no
> > idea whether there are other filesystems that are affected.
>
> It shouldn't silently break... It will refuse to compile. I renamed
> "memory_backed" to "capabilities".
This will silently break:
static struct backing_dev_info my_bdi = {
.ra_pages = MY_RA,
.unplug_io_fn = default_unplug_io_fn,
};
Miklos
-
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 1/2] BDI: Provide backing device capability information
Miklos Szeredi <[EMAIL PROTECTED]> wrote: > > The attached patch replaces backing_dev_info::memory_backed with > > capabilitied bitmap. The capabilities available include: > > Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY > and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way > out of tree filesystems that implicitly zero bdi->memory_backed > wouldn't _silently_ break. E.g. fuse does this, though it would not > actually break since it doesn't dirty any pages currently. I have no > idea whether there are other filesystems that are affected. It shouldn't silently break... It will refuse to compile. I renamed "memory_backed" to "capabilities". David - 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 1/2] BDI: Provide backing device capability information
> The attached patch replaces backing_dev_info::memory_backed with > capabilitied bitmap. The capabilities available include: Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way out of tree filesystems that implicitly zero bdi->memory_backed wouldn't _silently_ break. E.g. fuse does this, though it would not actually break since it doesn't dirty any pages currently. I have no idea whether there are other filesystems that are affected. Miklos - 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 1/2] BDI: Provide backing device capability information
The attached patch replaces backing_dev_info::memory_backed with capabilitied
bitmap. The capabilities available include:
(*) BDI_CAP_ACCOUNT_DIRTY
Set if the pages associated with this backing device should be tracked
by dirty page accounting.
(*) BDI_CAP_WRITEBACK_DIRTY
Set if dirty pages associated with this backing device should have
writepage() or writepages() invoked upon them to clean them.
(*) Capability markers that indicate what a backing device is capable of
with regard to memory mapping facilities. These flags indicate whether a
device can be mapped directly, whether it can be copied for a mapping,
and whether direct mappings can be read, written and/or executed. This
information is primarily aimed at improving no-MMU private mapping
support.
The patch alco provides convenience functions for determining the dirty-page
capabilities available on backing devices directly or on the backing devices
associated with a mapping. These are provided to keep line length down when
checking for the capabilities.
Signed-Off-By: David Howells <[EMAIL PROTECTED]>
---
warthog>diffstat -p1 memback-bdicap-2611rc4-2.diff
drivers/block/ll_rw_blk.c |2 +-
drivers/block/rd.c |6 +++---
drivers/char/mem.c |6 ++
fs/buffer.c |2 +-
fs/fs-writeback.c |4 ++--
fs/hugetlbfs/inode.c|2 +-
fs/ramfs/inode.c|3 ++-
fs/sysfs/inode.c|2 +-
include/linux/backing-dev.h | 41 -
mm/filemap.c|6 +++---
mm/page-writeback.c |6 +++---
mm/readahead.c |1 +
mm/shmem.c |4 ++--
mm/swap_state.c |2 +-
14 files changed, 67 insertions(+), 20 deletions(-)
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-07
13:34:16.068878317 +
@@ -25,13 +25,39 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned int capabilities; /* Device capabilities */
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - The first two flags control whether dirty pages will contribute to the
+ * VM's accounting and whether writepages() should be called for dirty pages
+ * (something that would not, for example, be appropriate for ramfs)
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_ACCOUNT_DIRTY 0x0001 /* Dirty pages should
contribute to accounting */
+#define BDI_CAP_WRITEBACK_DIRTY0x0002 /* Dirty pages should
be written back */
+#define BDI_CAP_MAP_COPY 0x0004 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0008 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP 0x0010 /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP 0x0020 /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP 0x0040 /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if defined(VM_MAYREAD) && \
+ (BDI_CAP_READ_MAP != VM_MAYREAD || \
+BDI_CAP_WRITE_MAP != VM_MAYWRITE || \
+BDI_CAP_EXEC_MAP != VM_MAYEXEC)
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
@@ -62,4 +88,17 @@ static inline int bdi_rw_congested(struc
(1 << BDI_write_congested));
}
+#define bdi_cap_writeback_dirty(bdi) \
+ ((bdi)->capabilities & BDI_CAP_WRITEBACK_DIRTY)
+
+#define bdi_cap_account_dirty(bdi) \
+ ((bdi)->capabilities & BDI_CAP_ACCOUNT_DIRTY)
+
+#define mapping_cap_writeback_dirty(mapping) \
+ bdi_cap_writeback_dirty((mapping)->backing_dev_info)
+
+#define mapping_cap_account_dirty(mapping) \
+ bdi_cap_account_dirty((mapping)->backing_dev_info)
+
+
#endif /* _LINUX_BACKING_DEV_H */
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
Re: [PATCH 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > > Making it unsigned long on a 32-bit machine will make no > > difference. Making it unsigned int on a 64-bit machine will waste four > > bytes. > > No, it won't waste any bytes at all. It won't save any either. > > But if someone comes along later and wants to add another int to that > structure, they won't know that your field was needlessly made a long, and > they'll then waste another eight bytes. Okay... Fair enough. David - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > Andrew Morton <[EMAIL PROTECTED]> wrote: > > > > Any particular reason? It's mixed in with other unsigned longs and > > > pointers > > > after all... > > > > Just that it's the natural wordsize of the machine, and uses less storage. > > Making it unsigned long on a 32-bit machine will make no difference. Making it > unsigned int on a 64-bit machine will waste four bytes. > No, it won't waste any bytes at all. It won't save any either. But if someone comes along later and wants to add another int to that structure, they won't know that your field was needlessly made a long, and they'll then waste another eight bytes. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > > Any particular reason? It's mixed in with other unsigned longs and pointers > > after all... > > Just that it's the natural wordsize of the machine, and uses less storage. Making it unsigned long on a 32-bit machine will make no difference. Making it unsigned int on a 64-bit machine will waste four bytes. David - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > Andrew Morton <[EMAIL PROTECTED]> wrote: > > > > So I should fold the two other bitfields back into the capabilities mask > > > and make it an unsigned long. > > > > I suppose so. Although unsigned int would be preferable. > > Any particular reason? It's mixed in with other unsigned longs and pointers > after all... Just that it's the natural wordsize of the machine, and uses less storage. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > > So I should fold the two other bitfields back into the capabilities mask > > and make it an unsigned long. > > I suppose so. Although unsigned int would be preferable. Any particular reason? It's mixed in with other unsigned longs and pointers after all... - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > Andrew Morton <[EMAIL PROTECTED]> wrote: > > > > Making these into bitfields would result in having to use three > variables > > > instead of just the one. > > > > Well let's do one or the other, and not have it half-and-half, please. > > So I should fold the two other bitfields back into the capabilities mask and > make it an unsigned long. I suppose so. Although unsigned int would be preferable. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > > Making these into bitfields would result in having to use three variables > > instead of just the one. > > Well let's do one or the other, and not have it half-and-half, please. So I should fold the two other bitfields back into the capabilities mask and make it an unsigned long. David - 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 1/2] BDI: Provide backing device capability information
Andrew Morton [EMAIL PROTECTED] wrote: Making these into bitfields would result in having to use three variables instead of just the one. Well let's do one or the other, and not have it half-and-half, please. So I should fold the two other bitfields back into the capabilities mask and make it an unsigned long. David - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: Andrew Morton [EMAIL PROTECTED] wrote: Making these into bitfields would result in having to use three variables instead of just the one. Well let's do one or the other, and not have it half-and-half, please. So I should fold the two other bitfields back into the capabilities mask and make it an unsigned long. I suppose so. Although unsigned int would be preferable. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton [EMAIL PROTECTED] wrote: So I should fold the two other bitfields back into the capabilities mask and make it an unsigned long. I suppose so. Although unsigned int would be preferable. Any particular reason? It's mixed in with other unsigned longs and pointers after all... - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: Andrew Morton [EMAIL PROTECTED] wrote: So I should fold the two other bitfields back into the capabilities mask and make it an unsigned long. I suppose so. Although unsigned int would be preferable. Any particular reason? It's mixed in with other unsigned longs and pointers after all... Just that it's the natural wordsize of the machine, and uses less storage. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton [EMAIL PROTECTED] wrote: Any particular reason? It's mixed in with other unsigned longs and pointers after all... Just that it's the natural wordsize of the machine, and uses less storage. Making it unsigned long on a 32-bit machine will make no difference. Making it unsigned int on a 64-bit machine will waste four bytes. David - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: Andrew Morton [EMAIL PROTECTED] wrote: Any particular reason? It's mixed in with other unsigned longs and pointers after all... Just that it's the natural wordsize of the machine, and uses less storage. Making it unsigned long on a 32-bit machine will make no difference. Making it unsigned int on a 64-bit machine will waste four bytes. No, it won't waste any bytes at all. It won't save any either. But if someone comes along later and wants to add another int to that structure, they won't know that your field was needlessly made a long, and they'll then waste another eight bytes. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton [EMAIL PROTECTED] wrote: Making it unsigned long on a 32-bit machine will make no difference. Making it unsigned int on a 64-bit machine will waste four bytes. No, it won't waste any bytes at all. It won't save any either. But if someone comes along later and wants to add another int to that structure, they won't know that your field was needlessly made a long, and they'll then waste another eight bytes. Okay... Fair enough. David - 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 1/2] BDI: Provide backing device capability information
The attached patch replaces backing_dev_info::memory_backed with capabilitied
bitmap. The capabilities available include:
(*) BDI_CAP_ACCOUNT_DIRTY
Set if the pages associated with this backing device should be tracked
by dirty page accounting.
(*) BDI_CAP_WRITEBACK_DIRTY
Set if dirty pages associated with this backing device should have
writepage() or writepages() invoked upon them to clean them.
(*) Capability markers that indicate what a backing device is capable of
with regard to memory mapping facilities. These flags indicate whether a
device can be mapped directly, whether it can be copied for a mapping,
and whether direct mappings can be read, written and/or executed. This
information is primarily aimed at improving no-MMU private mapping
support.
The patch alco provides convenience functions for determining the dirty-page
capabilities available on backing devices directly or on the backing devices
associated with a mapping. These are provided to keep line length down when
checking for the capabilities.
Signed-Off-By: David Howells [EMAIL PROTECTED]
---
warthogdiffstat -p1 memback-bdicap-2611rc4-2.diff
drivers/block/ll_rw_blk.c |2 +-
drivers/block/rd.c |6 +++---
drivers/char/mem.c |6 ++
fs/buffer.c |2 +-
fs/fs-writeback.c |4 ++--
fs/hugetlbfs/inode.c|2 +-
fs/ramfs/inode.c|3 ++-
fs/sysfs/inode.c|2 +-
include/linux/backing-dev.h | 41 -
mm/filemap.c|6 +++---
mm/page-writeback.c |6 +++---
mm/readahead.c |1 +
mm/shmem.c |4 ++--
mm/swap_state.c |2 +-
14 files changed, 67 insertions(+), 20 deletions(-)
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-07
13:34:16.068878317 +
@@ -25,13 +25,39 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned int capabilities; /* Device capabilities */
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - The first two flags control whether dirty pages will contribute to the
+ * VM's accounting and whether writepages() should be called for dirty pages
+ * (something that would not, for example, be appropriate for ramfs)
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_ACCOUNT_DIRTY 0x0001 /* Dirty pages should
contribute to accounting */
+#define BDI_CAP_WRITEBACK_DIRTY0x0002 /* Dirty pages should
be written back */
+#define BDI_CAP_MAP_COPY 0x0004 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0008 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP 0x0010 /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP 0x0020 /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP 0x0040 /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if defined(VM_MAYREAD) \
+ (BDI_CAP_READ_MAP != VM_MAYREAD || \
+BDI_CAP_WRITE_MAP != VM_MAYWRITE || \
+BDI_CAP_EXEC_MAP != VM_MAYEXEC)
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
@@ -62,4 +88,17 @@ static inline int bdi_rw_congested(struc
(1 BDI_write_congested));
}
+#define bdi_cap_writeback_dirty(bdi) \
+ ((bdi)-capabilities BDI_CAP_WRITEBACK_DIRTY)
+
+#define bdi_cap_account_dirty(bdi) \
+ ((bdi)-capabilities BDI_CAP_ACCOUNT_DIRTY)
+
+#define mapping_cap_writeback_dirty(mapping) \
+ bdi_cap_writeback_dirty((mapping)-backing_dev_info)
+
+#define mapping_cap_account_dirty(mapping) \
+ bdi_cap_account_dirty((mapping)-backing_dev_info)
+
+
#endif /* _LINUX_BACKING_DEV_H */
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
Re: [PATCH 1/2] BDI: Provide backing device capability information
The attached patch replaces backing_dev_info::memory_backed with capabilitied bitmap. The capabilities available include: Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way out of tree filesystems that implicitly zero bdi-memory_backed wouldn't _silently_ break. E.g. fuse does this, though it would not actually break since it doesn't dirty any pages currently. I have no idea whether there are other filesystems that are affected. Miklos - 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 1/2] BDI: Provide backing device capability information
Miklos Szeredi [EMAIL PROTECTED] wrote: The attached patch replaces backing_dev_info::memory_backed with capabilitied bitmap. The capabilities available include: Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way out of tree filesystems that implicitly zero bdi-memory_backed wouldn't _silently_ break. E.g. fuse does this, though it would not actually break since it doesn't dirty any pages currently. I have no idea whether there are other filesystems that are affected. It shouldn't silently break... It will refuse to compile. I renamed memory_backed to capabilities. David - 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 1/2] BDI: Provide backing device capability information
Wouldn't it be better to reverse the meaning of BDI_CAP_ACCOUNT_DIRTY
and BDI_CAP_WRITEBACK_DIRTY (BDI_CAP_NO_ACCOUNT_DIRTY...)? That way
out of tree filesystems that implicitly zero bdi-memory_backed
wouldn't _silently_ break. E.g. fuse does this, though it would not
actually break since it doesn't dirty any pages currently. I have no
idea whether there are other filesystems that are affected.
It shouldn't silently break... It will refuse to compile. I renamed
memory_backed to capabilities.
This will silently break:
static struct backing_dev_info my_bdi = {
.ra_pages = MY_RA,
.unplug_io_fn = default_unplug_io_fn,
};
Miklos
-
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 1/2] BDI: Provide backing device capability information
Miklos Szeredi [EMAIL PROTECTED] wrote:
It shouldn't silently break... It will refuse to compile. I renamed
memory_backed to capabilities.
This will silently break:
static struct backing_dev_info my_bdi = {
.ra_pages = MY_RA,
.unplug_io_fn = default_unplug_io_fn,
};
Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and
although it was in conjunction with turning the concepts into bitfields, it
still stands here.
David
-
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 1/2] BDI: Provide backing device capability information
Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and although it was in conjunction with turning the concepts into bitfields, it still stands here. OK, obviously Andrew has the final word in this. I just suggested that it might be safer to have the logic flipped back. Miklos - 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 1/2] BDI: Provide backing device capability information
Miklos Szeredi [EMAIL PROTECTED] wrote: Sorry, yes. Obvious. Ugh. Andrew Morton suggested flipping the logic, and although it was in conjunction with turning the concepts into bitfields, it still stands here. OK, obviously Andrew has the final word in this. I just suggested that it might be safer to have the logic flipped back. Experience indicates that it's safer to ignore anything I say on this topic. Yeah, it would be better to not flip the logic. - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > Andrew Morton <[EMAIL PROTECTED]> wrote: > > > > > > +#define BDI_CAP_MAP_COPY0x0001 /* Copy can be mapped > (MAP_PRIVATE) */ > > > +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped > directly (MAP_SHARED) */ > > > > Why not make these bitfields as well? > > Because I want to copy the capabilities mask (including these variables) into > a variable in the nommu mmap implementation and eliminate various bits from > that variable under certain conditions. > > Making these into bitfields would result in having to use three variables > instead of just the one. Well let's do one or the other, and not have it half-and-half, please. - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > Andrew Morton <[EMAIL PROTECTED]> wrote: > > > Yup. In this application the fields are initialised once (usually at > > compile time) and are never modified. > > That's not exactly so. The block layer appears to modify them. See > blk_queue_make_request() in ll_rw_blk.c. > That's a (strangely named) once-off setup thing. - 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 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > Yup. In this application the fields are initialised once (usually at > compile time) and are never modified. That's not exactly so. The block layer appears to modify them. See blk_queue_make_request() in ll_rw_blk.c. David - 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 1/2] BDI: Provide backing device capability information
Andrew Morton <[EMAIL PROTECTED]> wrote: > > > +#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped > > (MAP_PRIVATE) */ > > +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly > > (MAP_SHARED) */ > > Why not make these bitfields as well? Because I want to copy the capabilities mask (including these variables) into a variable in the nommu mmap implementation and eliminate various bits from that variable under certain conditions. Making these into bitfields would result in having to use three variables instead of just the one. David - 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 1/2] BDI: Provide backing device capability information
Andrew Morton [EMAIL PROTECTED] wrote: +#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped (MAP_PRIVATE) */ +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly (MAP_SHARED) */ Why not make these bitfields as well? Because I want to copy the capabilities mask (including these variables) into a variable in the nommu mmap implementation and eliminate various bits from that variable under certain conditions. Making these into bitfields would result in having to use three variables instead of just the one. David - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: Andrew Morton [EMAIL PROTECTED] wrote: Yup. In this application the fields are initialised once (usually at compile time) and are never modified. That's not exactly so. The block layer appears to modify them. See blk_queue_make_request() in ll_rw_blk.c. That's a (strangely named) once-off setup thing. - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: Andrew Morton [EMAIL PROTECTED] wrote: +#define BDI_CAP_MAP_COPY0x0001 /* Copy can be mapped (MAP_PRIVATE) */ +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly (MAP_SHARED) */ Why not make these bitfields as well? Because I want to copy the capabilities mask (including these variables) into a variable in the nommu mmap implementation and eliminate various bits from that variable under certain conditions. Making these into bitfields would result in having to use three variables instead of just the one. Well let's do one or the other, and not have it half-and-half, please. - 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 1/2] BDI: Provide backing device capability information
David Howells <[EMAIL PROTECTED]> wrote: > > > The attached patch does two things: > > (1) It gets rid of backing_dev_info::memory_backed and replaces it with a > pair of boolean values: > > (*) dirty_memory_acct > > True if the pages associated with this backing device should be > tracked by dirty page accounting. > > (*) writeback_if_dirty > > True if the pages associated with this backing device should have > writepage() or writepages() invoked upon them to clean them. Cool, thanks. > (2) It adds a backing device capability mask that indicates what a backing > device is capable of; currently only in regard to memory mapping > facilities. These flags indicate whether a device can be mapped directly, > whether it can be copied for a mapping, and whether direct mappings can > be read, written and/or executed. This information is primarily aimed at > improving no-MMU private mapping support. > > ... > +#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped > (MAP_PRIVATE) */ > +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly > (MAP_SHARED) */ Why not make these bitfields as well? - 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 1/2] BDI: Provide backing device capability information
Linus Torvalds <[EMAIL PROTECTED]> wrote: > > On Wed, 2 Mar 2005, Andrew Morton wrote: > > > > Why not make these bitfields as well? > > Side note: bitfields aren't exactly wonderful. Yup. In this application the fields are initialised once (usually at compile time) and are never modified. So the compiler should be able to generate the same code as it would with an open-coded bit test. Which is about the only situation where we should use bitfields, IMO. That being said, there aren't many backing_dev_info's in a system, so we won't be saving much memory. Some architectures will presumably generate faster code with plain old integers. You'd only ever lose if the backing_dev_info happened to straddle a cacheline boundary. It's marginal. - 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 1/2] BDI: Provide backing device capability information
On Wed, 2 Mar 2005, Andrew Morton wrote: > > Why not make these bitfields as well? Side note: bitfields aren't exactly wonderful. They tend to generate worse code, and they make it much harder to work with combinations of flags (both testing and initializing). They also have architecture-specific bit-order and packing issues, which means that when coupled with device drivers etc they can be a major pain in the derriere. Even apart from those issues, they can't sanely be atomically accessed, so in many cases they just aren't a good thing. In contrast, just using a flag word and explicit bitmask has none of those problems, and it's often easy to abstract out things with a macro and/or inline function. So don't go for bitfields "just because". It's generally a good idea to _require_ that the code in question has none of the potential problem spots even in _theory_, and in addition also show that bitfields really make the code look nicer. The downsides really can be that nasty. Linus - 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/
[PATCH 1/2] BDI: Provide backing device capability information
The attached patch does two things:
(1) It gets rid of backing_dev_info::memory_backed and replaces it with a
pair of boolean values:
(*) dirty_memory_acct
True if the pages associated with this backing device should be
tracked by dirty page accounting.
(*) writeback_if_dirty
True if the pages associated with this backing device should have
writepage() or writepages() invoked upon them to clean them.
(2) It adds a backing device capability mask that indicates what a backing
device is capable of; currently only in regard to memory mapping
facilities. These flags indicate whether a device can be mapped directly,
whether it can be copied for a mapping, and whether direct mappings can
be read, written and/or executed. This information is primarily aimed at
improving no-MMU private mapping support.
Signed-Off-By: David Howells <[EMAIL PROTECTED]>
---
warthog>diffstat -p1 memback-bdicap-2611rc4.diff
drivers/block/ll_rw_blk.c |3 ++-
drivers/block/rd.c | 14 --
drivers/char/mem.c |6 ++
fs/buffer.c |2 +-
fs/fs-writeback.c |4 ++--
fs/hugetlbfs/inode.c|5 +++--
fs/ramfs/inode.c|7 +--
fs/sysfs/inode.c|5 +++--
include/linux/backing-dev.h | 22 +-
mm/filemap.c|6 +++---
mm/page-writeback.c |6 +++---
mm/readahead.c |8 +---
mm/shmem.c |7 ---
mm/swap_state.c |5 +++--
14 files changed, 69 insertions(+), 31 deletions(-)
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-02
21:06:31.733471683 +
@@ -25,13 +25,33 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned short capabilities; /* Device capabilities */
+ unsigned dirty_memory_acct : 1; /* Contributes to dirty memory
accounting */
+ unsigned writeback_if_dirty : 1; /* Dirty memory should be written back
*/
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP VM_MAYREAD /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP VM_MAYWRITE /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP VM_MAYEXEC /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if (BDI_CAP_MAP_DIRECT | BDI_CAP_COPY_FOR_MAP) & BDI_CAP_VMFLAGS
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c 2005-02-14
12:18:23.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c 2005-03-02
18:19:33.0 +
@@ -238,7 +238,8 @@ void blk_queue_make_request(request_queu
q->make_request_fn = mfn;
q->backing_dev_info.ra_pages = (VM_MAX_READAHEAD * 1024) /
PAGE_CACHE_SIZE;
q->backing_dev_info.state = 0;
- q->backing_dev_info.memory_backed = 0;
+ q->backing_dev_info.writeback_if_dirty = 1;
+ q->backing_dev_info.dirty_memory_acct = 1;
blk_queue_max_sectors(q, MAX_SECTORS);
blk_queue_hardsect_size(q, 512);
blk_queue_dma_alignment(q, 511);
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/rd.c
linux-2.6.11-rc4-memback/drivers/block/rd.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/rd.c2005-01-04
11:13:05.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/rd.c 2005-03-02 18:57:41.0
+
@@ -324,9 +324,10 @@ static int rd_ioctl(struct inode *inode,
*
[PATCH 1/2] BDI: Provide backing device capability information
The attached patch does two things:
(1) It gets rid of backing_dev_info::memory_backed and replaces it with a
pair of boolean values:
(*) dirty_memory_acct
True if the pages associated with this backing device should be
tracked by dirty page accounting.
(*) writeback_if_dirty
True if the pages associated with this backing device should have
writepage() or writepages() invoked upon them to clean them.
(2) It adds a backing device capability mask that indicates what a backing
device is capable of; currently only in regard to memory mapping
facilities. These flags indicate whether a device can be mapped directly,
whether it can be copied for a mapping, and whether direct mappings can
be read, written and/or executed. This information is primarily aimed at
improving no-MMU private mapping support.
Signed-Off-By: David Howells [EMAIL PROTECTED]
---
warthogdiffstat -p1 memback-bdicap-2611rc4.diff
drivers/block/ll_rw_blk.c |3 ++-
drivers/block/rd.c | 14 --
drivers/char/mem.c |6 ++
fs/buffer.c |2 +-
fs/fs-writeback.c |4 ++--
fs/hugetlbfs/inode.c|5 +++--
fs/ramfs/inode.c|7 +--
fs/sysfs/inode.c|5 +++--
include/linux/backing-dev.h | 22 +-
mm/filemap.c|6 +++---
mm/page-writeback.c |6 +++---
mm/readahead.c |8 +---
mm/shmem.c |7 ---
mm/swap_state.c |5 +++--
14 files changed, 69 insertions(+), 31 deletions(-)
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
linux-2.6.11-rc4-memback/include/linux/backing-dev.h
--- /warthog/kernels/linux-2.6.11-rc4/include/linux/backing-dev.h
2004-06-18 13:44:05.0 +0100
+++ linux-2.6.11-rc4-memback/include/linux/backing-dev.h2005-03-02
21:06:31.733471683 +
@@ -25,13 +25,33 @@ typedef int (congested_fn)(void *, int);
struct backing_dev_info {
unsigned long ra_pages; /* max readahead in PAGE_CACHE_SIZE units */
unsigned long state;/* Always use atomic bitops on this */
- int memory_backed; /* Cannot clean pages with writepage */
+ unsigned short capabilities; /* Device capabilities */
+ unsigned dirty_memory_acct : 1; /* Contributes to dirty memory
accounting */
+ unsigned writeback_if_dirty : 1; /* Dirty memory should be written back
*/
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
void (*unplug_io_fn)(struct backing_dev_info *, struct page *);
void *unplug_io_data;
};
+
+/*
+ * Flags in backing_dev_info::capability
+ * - These flags let !MMU mmap() govern direct device mapping vs immediate
+ * copying more easily for MAP_PRIVATE, especially for ROM filesystems
+ */
+#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped
(MAP_PRIVATE) */
+#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly
(MAP_SHARED) */
+#define BDI_CAP_READ_MAP VM_MAYREAD /* Can be mapped for reading */
+#define BDI_CAP_WRITE_MAP VM_MAYWRITE /* Can be mapped for writing */
+#define BDI_CAP_EXEC_MAP VM_MAYEXEC /* Can be mapped for execution
*/
+#define BDI_CAP_VMFLAGS \
+ (BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP)
+
+#if (BDI_CAP_MAP_DIRECT | BDI_CAP_COPY_FOR_MAP) BDI_CAP_VMFLAGS
+#error please change backing_dev_info::capabilities flags
+#endif
+
extern struct backing_dev_info default_backing_dev_info;
void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c
linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/ll_rw_blk.c 2005-02-14
12:18:23.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/ll_rw_blk.c 2005-03-02
18:19:33.0 +
@@ -238,7 +238,8 @@ void blk_queue_make_request(request_queu
q-make_request_fn = mfn;
q-backing_dev_info.ra_pages = (VM_MAX_READAHEAD * 1024) /
PAGE_CACHE_SIZE;
q-backing_dev_info.state = 0;
- q-backing_dev_info.memory_backed = 0;
+ q-backing_dev_info.writeback_if_dirty = 1;
+ q-backing_dev_info.dirty_memory_acct = 1;
blk_queue_max_sectors(q, MAX_SECTORS);
blk_queue_hardsect_size(q, 512);
blk_queue_dma_alignment(q, 511);
diff -uNrp /warthog/kernels/linux-2.6.11-rc4/drivers/block/rd.c
linux-2.6.11-rc4-memback/drivers/block/rd.c
--- /warthog/kernels/linux-2.6.11-rc4/drivers/block/rd.c2005-01-04
11:13:05.0 +
+++ linux-2.6.11-rc4-memback/drivers/block/rd.c 2005-03-02 18:57:41.0
+
@@ -324,9 +324,10 @@ static int rd_ioctl(struct inode *inode,
* writeback and
Re: [PATCH 1/2] BDI: Provide backing device capability information
On Wed, 2 Mar 2005, Andrew Morton wrote: Why not make these bitfields as well? Side note: bitfields aren't exactly wonderful. They tend to generate worse code, and they make it much harder to work with combinations of flags (both testing and initializing). They also have architecture-specific bit-order and packing issues, which means that when coupled with device drivers etc they can be a major pain in the derriere. Even apart from those issues, they can't sanely be atomically accessed, so in many cases they just aren't a good thing. In contrast, just using a flag word and explicit bitmask has none of those problems, and it's often easy to abstract out things with a macro and/or inline function. So don't go for bitfields just because. It's generally a good idea to _require_ that the code in question has none of the potential problem spots even in _theory_, and in addition also show that bitfields really make the code look nicer. The downsides really can be that nasty. Linus - 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 1/2] BDI: Provide backing device capability information
Linus Torvalds [EMAIL PROTECTED] wrote: On Wed, 2 Mar 2005, Andrew Morton wrote: Why not make these bitfields as well? Side note: bitfields aren't exactly wonderful. Yup. In this application the fields are initialised once (usually at compile time) and are never modified. So the compiler should be able to generate the same code as it would with an open-coded bit test. Which is about the only situation where we should use bitfields, IMO. That being said, there aren't many backing_dev_info's in a system, so we won't be saving much memory. Some architectures will presumably generate faster code with plain old integers. You'd only ever lose if the backing_dev_info happened to straddle a cacheline boundary. It's marginal. - 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 1/2] BDI: Provide backing device capability information
David Howells [EMAIL PROTECTED] wrote: The attached patch does two things: (1) It gets rid of backing_dev_info::memory_backed and replaces it with a pair of boolean values: (*) dirty_memory_acct True if the pages associated with this backing device should be tracked by dirty page accounting. (*) writeback_if_dirty True if the pages associated with this backing device should have writepage() or writepages() invoked upon them to clean them. Cool, thanks. (2) It adds a backing device capability mask that indicates what a backing device is capable of; currently only in regard to memory mapping facilities. These flags indicate whether a device can be mapped directly, whether it can be copied for a mapping, and whether direct mappings can be read, written and/or executed. This information is primarily aimed at improving no-MMU private mapping support. ... +#define BDI_CAP_MAP_COPY 0x0001 /* Copy can be mapped (MAP_PRIVATE) */ +#define BDI_CAP_MAP_DIRECT 0x0002 /* Can be mapped directly (MAP_SHARED) */ Why not make these bitfields as well? - 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/
