* RFC: Changes to fs/buffer.c?
@ 2004-10-29 14:28 Anton Altaparmakov
2004-10-29 20:34 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Anton Altaparmakov @ 2004-10-29 14:28 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: linux-kernel
Hi Andrew, hi Linus,
I would like to use create_buffers() and __set_page_buffers() in ntfs but
both are static functions in fs/buffer.c. Is it ok to export
create_buffers() and to make __set_page_buffers() static inline and move
it to include/linux/buffer.h? In other words, is the below patch
acceptable?
If you are curious why I need these, it is because
mark_ntfs_record_dirty() needs to create buffers if they are not present
so that it can set only some of them dirty and create_empty_buffers() sets
all buffers dirty which is not useful. Also create_empty_buffers() relies
on the page being locked which is not necessarily the case when
mark_ntfs_record_dirty() is called. This allows me to only mark parts of
a page as dirty which means ->writepage() does not need to write the whole
page but only the dirty ntfs records in the page. The function I am
talking about is at the end of this email if you want to see it.
Thanks a lot in advance.
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
--- ntfs-2.6-devel/include/linux/buffer_head.h.old 2004-10-29 14:38:11.543109318 +0100
+++ ntfs-2.6-devel/include/linux/buffer_head.h 2004-10-29 14:38:48.718192623 +0100
@@ -10,6 +10,7 @@
#include <linux/types.h>
#include <linux/fs.h>
#include <linux/linkage.h>
+#include <linux/pagemap.h>
#include <linux/wait.h>
#include <asm/atomic.h>
@@ -136,6 +137,8 @@ void init_buffer(struct buffer_head *, b
void set_bh_page(struct buffer_head *bh,
struct page *page, unsigned long offset);
int try_to_free_buffers(struct page *);
+struct buffer_head *create_buffers(struct page *page, unsigned long size,
+ int retry);
void create_empty_buffers(struct page *, unsigned long,
unsigned long b_state);
void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
@@ -205,6 +208,14 @@ int nobh_truncate_page(struct address_sp
* inline definitions
*/
+static inline void __set_page_buffers(struct page *page,
+ struct buffer_head *head)
+{
+ page_cache_get(page);
+ SetPagePrivate(page);
+ page->private = (unsigned long)head;
+}
+
static inline void get_bh(struct buffer_head *bh)
{
atomic_inc(&bh->b_count);
--- ntfs-2.6-devel/fs/buffer.c.old 2004-10-29 12:50:57.271105103 +0100
+++ ntfs-2.6-devel/fs/buffer.c 2004-10-29 14:35:29.254207332 +0100
@@ -91,14 +91,6 @@ void __wait_on_buffer(struct buffer_head
}
static void
-__set_page_buffers(struct page *page, struct buffer_head *head)
-{
- page_cache_get(page);
- SetPagePrivate(page);
- page->private = (unsigned long)head;
-}
-
-static void
__clear_page_buffers(struct page *page)
{
ClearPagePrivate(page);
@@ -1013,8 +1005,8 @@ int remove_inode_buffers(struct inode *i
* The retry flag is used to differentiate async IO (paging, swapping)
* which may not fail from ordinary buffer allocations.
*/
-static struct buffer_head *
-create_buffers(struct page * page, unsigned long size, int retry)
+struct buffer_head *create_buffers(struct page *page, unsigned long size,
+ int retry)
{
struct buffer_head *bh, *head;
long offset;
@@ -1072,6 +1064,7 @@ no_grow:
free_more_memory();
goto try_again;
}
+EXPORT_SYMBOL(create_buffers);
static inline void
link_dev_buffers(struct page *page, struct buffer_head *head)
------------- this is mark_ntfs_record_dirty() --------------
/**
* mark_ntfs_record_dirty - mark an ntfs record dirty
* @page: page containing the ntfs record to mark dirty
* @ofs: byte offset within @page at which the ntfs record begins
*
* Set the buffers and the page in which the ntfs record is located dirty.
*
* The latter also marks the vfs inode the ntfs record belongs to dirty
* (I_DIRTY_PAGES only).
*
* If the page does not have buffers, we create them and set them uptodate.
* The page may not be locked which is why we need to handle the buffers under
* the mapping->private_lock. Once the buffers are marked dirty we no longer
* need the lock since try_to_free_buffers() does not free dirty buffers.
*/
void mark_ntfs_record_dirty(struct page *page, const unsigned int ofs) {
struct address_space *mapping = page->mapping;
ntfs_inode *ni = NTFS_I(mapping->host);
struct buffer_head *bh, *head, *buffers_to_free = NULL;
unsigned int end, bh_size, bh_ofs;
BUG_ON(!PageUptodate(page));
end = ofs + ni->itype.index.block_size;
bh_size = 1 << VFS_I(ni)->i_blkbits;
spin_lock(&mapping->private_lock);
if (unlikely(!page_has_buffers(page))) {
spin_unlock(&mapping->private_lock);
bh = head = create_buffers(page, bh_size, 1);
spin_lock(&mapping->private_lock);
if (likely(!page_has_buffers(page))) {
struct buffer_head *tail;
do {
set_buffer_uptodate(bh);
tail = bh;
bh = bh->b_this_page;
} while (bh);
tail->b_this_page = head;
__set_page_buffers(page, head);
} else
buffers_to_free = bh;
}
bh = head = page_buffers(page);
do {
bh_ofs = bh_offset(bh);
if (bh_ofs + bh_size <= ofs)
continue;
if (unlikely(bh_ofs >= end))
break;
set_buffer_dirty(bh);
} while ((bh = bh->b_this_page) != head);
spin_unlock(&mapping->private_lock);
__set_page_dirty_nobuffers(page);
if (unlikely(buffers_to_free)) {
do {
bh = buffers_to_free->b_this_page;
free_buffer_head(buffers_to_free);
buffers_to_free = bh;
} while (buffers_to_free);
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: RFC: Changes to fs/buffer.c? 2004-10-29 14:28 RFC: Changes to fs/buffer.c? Anton Altaparmakov @ 2004-10-29 20:34 ` Andrew Morton 2004-10-29 20:45 ` Linus Torvalds 2004-10-29 21:46 ` Anton Altaparmakov 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2004-10-29 20:34 UTC (permalink / raw) To: Anton Altaparmakov; +Cc: torvalds, linux-kernel Anton Altaparmakov <aia21@cam.ac.uk> wrote: > > Is it ok to export > create_buffers() and to make __set_page_buffers() static inline and move > it to include/linux/buffer.h? ho, hum - if you must ;) I'd be inclined to rename it to attach_page_buffers() or something though - create_buffers() is a bit generic-sounding. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: Changes to fs/buffer.c? 2004-10-29 20:34 ` Andrew Morton @ 2004-10-29 20:45 ` Linus Torvalds 2004-10-29 21:48 ` Anton Altaparmakov 2004-10-29 21:46 ` Anton Altaparmakov 1 sibling, 1 reply; 6+ messages in thread From: Linus Torvalds @ 2004-10-29 20:45 UTC (permalink / raw) To: Andrew Morton; +Cc: Anton Altaparmakov, linux-kernel On Fri, 29 Oct 2004, Andrew Morton wrote: > > Anton Altaparmakov <aia21@cam.ac.uk> wrote: > > > > Is it ok to export > > create_buffers() and to make __set_page_buffers() static inline and move > > it to include/linux/buffer.h? > > ho, hum - if you must ;) > > I'd be inclined to rename it to attach_page_buffers() or something though - > create_buffers() is a bit generic-sounding. Also, I think we should at least start out limiting it to GPL-only usage. Those page buffers are pretty intertwined with the VM usage, I'd hate to see people think this is some kind of external interface.. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: Changes to fs/buffer.c? 2004-10-29 20:45 ` Linus Torvalds @ 2004-10-29 21:48 ` Anton Altaparmakov 0 siblings, 0 replies; 6+ messages in thread From: Anton Altaparmakov @ 2004-10-29 21:48 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel On Fri, 29 Oct 2004, Linus Torvalds wrote: > On Fri, 29 Oct 2004, Andrew Morton wrote: > > Anton Altaparmakov <aia21@cam.ac.uk> wrote: > > > > > > Is it ok to export > > > create_buffers() and to make __set_page_buffers() static inline and move > > > it to include/linux/buffer.h? > > > > ho, hum - if you must ;) > > > > I'd be inclined to rename it to attach_page_buffers() or something though - > > create_buffers() is a bit generic-sounding. > > Also, I think we should at least start out limiting it to GPL-only usage. > Those page buffers are pretty intertwined with the VM usage, I'd hate to > see people think this is some kind of external interface.. Yes, sure. I will use the _GPL version for the symbol export. I only used the non-GPL-only form since create_empty_buffers() is simply exported and create_buffers() is kind of the same thing but does less... Best regards, Anton -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: Changes to fs/buffer.c? 2004-10-29 20:34 ` Andrew Morton 2004-10-29 20:45 ` Linus Torvalds @ 2004-10-29 21:46 ` Anton Altaparmakov 2004-11-01 22:42 ` [PATCH 2.6] " Anton Altaparmakov 1 sibling, 1 reply; 6+ messages in thread From: Anton Altaparmakov @ 2004-10-29 21:46 UTC (permalink / raw) To: Andrew Morton; +Cc: torvalds, linux-kernel On Fri, 29 Oct 2004, Andrew Morton wrote: > Anton Altaparmakov <aia21@cam.ac.uk> wrote: > > > > Is it ok to export > > create_buffers() and to make __set_page_buffers() static inline and move > > it to include/linux/buffer.h? > > ho, hum - if you must ;) I don't have to. It is very easy for me to take a copy of each of these and stick it in fs/ntfs/aops.c. Works out of the box. Just seems silly to proliferate code like that rather than to make use of existing functions... Would you not agree? If not, I will just copy the functions and that's that. > I'd be inclined to rename it to attach_page_buffers() or something though - > create_buffers() is a bit generic-sounding. create_empty_buffers() which is already exported and used by pretty much all fs is just as generic sounding... However renaming create_buffers() to alloc_page_buffers() might be better as it doesn't actually attach the buffers to the page. You still need the __set_page_buffers() to do that. Perhaps you meant to rename __set_page_buffers() to attach_page_buffers()? Its time for bed now but I will cook up a new patch tomorrow or Monday depending on when I get some time on the computer... Cheers, Anton -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2.6] Re: RFC: Changes to fs/buffer.c? 2004-10-29 21:46 ` Anton Altaparmakov @ 2004-11-01 22:42 ` Anton Altaparmakov 0 siblings, 0 replies; 6+ messages in thread From: Anton Altaparmakov @ 2004-11-01 22:42 UTC (permalink / raw) To: Andrew Morton, Linus Torvalds; +Cc: linux-kernel Hi Andrew, Hi Linus, On Fri, 29 Oct 2004, Anton Altaparmakov wrote: > On Fri, 29 Oct 2004, Andrew Morton wrote: > > Anton Altaparmakov <aia21@cam.ac.uk> wrote: > > > > > > Is it ok to export > > > create_buffers() and to make __set_page_buffers() static inline and move > > > it to include/linux/buffer.h? > > > > ho, hum - if you must ;) Ok, below is the patch, as requested I renamed the functions to more descriptive names: create_buffers -> alloc_page_buffers __set_page_buffers -> attach_page_buffers And I added a EXPORT_SYMBOL_GPL for alloc_page_buffers and made attach_page_buffers static inline and moved it to <linux/buffer_head.h>. Assuming you are both happy with it, please apply. Thanks! Signed-off-by: Anton Altaparmakov <aia21@cantab.net> Best regards, Anton PS. If you prefer me to submit this as part of the next ntfs update just let me know... -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/ ---------- buffer-stuff2.diff ---------- --- ntfs-2.6-devel/include/linux/buffer_head.h.old 2004-11-01 22:30:58.854166673 +0000 +++ ntfs-2.6-devel/include/linux/buffer_head.h 2004-11-01 22:27:22.722940892 +0000 @@ -10,6 +10,7 @@ #include <linux/types.h> #include <linux/fs.h> #include <linux/linkage.h> +#include <linux/pagemap.h> #include <linux/wait.h> #include <asm/atomic.h> @@ -136,6 +137,8 @@ void init_buffer(struct buffer_head *, b void set_bh_page(struct buffer_head *bh, struct page *page, unsigned long offset); int try_to_free_buffers(struct page *); +struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size, + int retry); void create_empty_buffers(struct page *, unsigned long, unsigned long b_state); void end_buffer_read_sync(struct buffer_head *bh, int uptodate); @@ -205,6 +208,14 @@ int nobh_truncate_page(struct address_sp * inline definitions */ +static inline void attach_page_buffers(struct page *page, + struct buffer_head *head) +{ + page_cache_get(page); + SetPagePrivate(page); + page->private = (unsigned long)head; +} + static inline void get_bh(struct buffer_head *bh) { atomic_inc(&bh->b_count); --- ntfs-2.6-devel/fs/buffer.c.old 2004-11-01 22:29:29.423590168 +0000 +++ ntfs-2.6-devel/fs/buffer.c 2004-11-01 22:27:57.582267689 +0000 @@ -91,14 +91,6 @@ void __wait_on_buffer(struct buffer_head } static void -__set_page_buffers(struct page *page, struct buffer_head *head) -{ - page_cache_get(page); - SetPagePrivate(page); - page->private = (unsigned long)head; -} - -static void __clear_page_buffers(struct page *page) { ClearPagePrivate(page); @@ -1013,8 +1005,8 @@ int remove_inode_buffers(struct inode *i * The retry flag is used to differentiate async IO (paging, swapping) * which may not fail from ordinary buffer allocations. */ -static struct buffer_head * -create_buffers(struct page * page, unsigned long size, int retry) +struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size, + int retry) { struct buffer_head *bh, *head; long offset; @@ -1072,6 +1064,7 @@ no_grow: free_more_memory(); goto try_again; } +EXPORT_SYMBOL_GPL(alloc_page_buffers); static inline void link_dev_buffers(struct page *page, struct buffer_head *head) @@ -1084,7 +1077,7 @@ link_dev_buffers(struct page *page, stru bh = bh->b_this_page; } while (bh); tail->b_this_page = head; - __set_page_buffers(page, head); + attach_page_buffers(page, head); } /* @@ -1145,7 +1138,7 @@ grow_dev_page(struct block_device *bdev, /* * Allocate some buffers for this page */ - bh = create_buffers(page, size, 0); + bh = alloc_page_buffers(page, size, 0); if (!bh) goto failed; @@ -1651,7 +1644,7 @@ void create_empty_buffers(struct page *p { struct buffer_head *bh, *head, *tail; - head = create_buffers(page, blocksize, 1); + head = alloc_page_buffers(page, blocksize, 1); bh = head; do { bh->b_state |= b_state; @@ -1671,7 +1664,7 @@ void create_empty_buffers(struct page *p bh = bh->b_this_page; } while (bh != head); } - __set_page_buffers(page, head); + attach_page_buffers(page, head); spin_unlock(&page->mapping->private_lock); } EXPORT_SYMBOL(create_empty_buffers); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-11-02 4:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-10-29 14:28 RFC: Changes to fs/buffer.c? Anton Altaparmakov 2004-10-29 20:34 ` Andrew Morton 2004-10-29 20:45 ` Linus Torvalds 2004-10-29 21:48 ` Anton Altaparmakov 2004-10-29 21:46 ` Anton Altaparmakov 2004-11-01 22:42 ` [PATCH 2.6] " Anton Altaparmakov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.