* [PATCH v2 1/8] doc: Improve the description of __folio_mark_dirty
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 2/8] buffer: Add kernel-doc for block_dirty_folio() Matthew Wilcox (Oracle)
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
I've learned why it's safe to call __folio_mark_dirty() from
mark_buffer_dirty() without holding the folio lock, so update
the description to explain why.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
mm/page-writeback.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index cd4e4ae77c40..f09179fca2cf 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2652,11 +2652,15 @@ void folio_account_cleaned(struct folio *folio, struct bdi_writeback *wb)
* If warn is true, then emit a warning if the folio is not uptodate and has
* not been truncated.
*
- * The caller must hold folio_memcg_lock(). Most callers have the folio
- * locked. A few have the folio blocked from truncation through other
- * means (eg zap_vma_pages() has it mapped and is holding the page table
- * lock). This can also be called from mark_buffer_dirty(), which I
- * cannot prove is always protected against truncate.
+ * The caller must hold folio_memcg_lock(). It is the caller's
+ * responsibility to prevent the folio from being truncated while
+ * this function is in progress, although it may have been truncated
+ * before this function is called. Most callers have the folio locked.
+ * A few have the folio blocked from truncation through other means (e.g.
+ * zap_vma_pages() has it mapped and is holding the page table lock).
+ * When called from mark_buffer_dirty(), the filesystem should hold a
+ * reference to the buffer_head that is being marked dirty, which causes
+ * try_to_free_buffers() to fail.
*/
void __folio_mark_dirty(struct folio *folio, struct address_space *mapping,
int warn)
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/8] buffer: Add kernel-doc for block_dirty_folio()
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 1/8] doc: Improve the description of __folio_mark_dirty Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-10 14:12 ` Pankaj Raghav (Samsung)
2024-01-09 14:33 ` [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers() Matthew Wilcox (Oracle)
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
Turn the excellent documentation for this function into kernel-doc.
Replace 'page' with 'folio' and make a few other minor updates.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 55 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index d3bcf601d3e5..071f01b28c90 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -687,30 +687,37 @@ void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode)
}
EXPORT_SYMBOL(mark_buffer_dirty_inode);
-/*
- * Add a page to the dirty page list.
- *
- * It is a sad fact of life that this function is called from several places
- * deeply under spinlocking. It may not sleep.
- *
- * If the page has buffers, the uptodate buffers are set dirty, to preserve
- * dirty-state coherency between the page and the buffers. It the page does
- * not have buffers then when they are later attached they will all be set
- * dirty.
- *
- * The buffers are dirtied before the page is dirtied. There's a small race
- * window in which a writepage caller may see the page cleanness but not the
- * buffer dirtiness. That's fine. If this code were to set the page dirty
- * before the buffers, a concurrent writepage caller could clear the page dirty
- * bit, see a bunch of clean buffers and we'd end up with dirty buffers/clean
- * page on the dirty page list.
- *
- * We use i_private_lock to lock against try_to_free_buffers while using the
- * page's buffer list. Also use this to protect against clean buffers being
- * added to the page after it was set dirty.
- *
- * FIXME: may need to call ->reservepage here as well. That's rather up to the
- * address_space though.
+/**
+ * block_dirty_folio - Mark a folio as dirty.
+ * @mapping: The address space containing this folio.
+ * @folio: The folio to mark dirty.
+ *
+ * Filesystems which use buffer_heads can use this function as their
+ * ->dirty_folio implementation. Some filesystems need to do a little
+ * work before calling this function. Filesystems which do not use
+ * buffer_heads should call filemap_dirty_folio() instead.
+ *
+ * If the folio has buffers, the uptodate buffers are set dirty, to
+ * preserve dirty-state coherency between the folio and the buffers.
+ * Buffers added to a dirty folio are created dirty.
+ *
+ * The buffers are dirtied before the folio is dirtied. There's a small
+ * race window in which writeback may see the folio cleanness but not the
+ * buffer dirtiness. That's fine. If this code were to set the folio
+ * dirty before the buffers, writeback could clear the folio dirty flag,
+ * see a bunch of clean buffers and we'd end up with dirty buffers/clean
+ * folio on the dirty folio list.
+ *
+ * We use i_private_lock to lock against try_to_free_buffers() while
+ * using the folio's buffer list. This also prevents clean buffers
+ * being added to the folio after it was set dirty.
+ *
+ * Context: May only be called from process context. Does not sleep.
+ * Caller must ensure that @folio cannot be truncated during this call,
+ * typically by holding the folio lock or having a page in the folio
+ * mapped and holding the page table lock.
+ *
+ * Return: True if the folio was dirtied; false if it was already dirtied.
*/
bool block_dirty_folio(struct address_space *mapping, struct folio *folio)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers()
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 1/8] doc: Improve the description of __folio_mark_dirty Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 2/8] buffer: Add kernel-doc for block_dirty_folio() Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-10 14:21 ` Pankaj Raghav (Samsung)
2024-01-11 3:22 ` Randy Dunlap
2024-01-09 14:33 ` [PATCH v2 4/8] buffer: Fix __bread and __bread_gfp kernel-doc Matthew Wilcox (Oracle)
` (4 subsequent siblings)
7 siblings, 2 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
The documentation for this function has become separated from it over
time; move it to the right place and turn it into kernel-doc. Mild
editing of the content to make it more about what the function does, and
less about how it does it.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 44 ++++++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 071f01b28c90..25861241657f 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2864,26 +2864,6 @@ int sync_dirty_buffer(struct buffer_head *bh)
}
EXPORT_SYMBOL(sync_dirty_buffer);
-/*
- * try_to_free_buffers() checks if all the buffers on this particular folio
- * are unused, and releases them if so.
- *
- * Exclusion against try_to_free_buffers may be obtained by either
- * locking the folio or by holding its mapping's i_private_lock.
- *
- * If the folio is dirty but all the buffers are clean then we need to
- * be sure to mark the folio clean as well. This is because the folio
- * may be against a block device, and a later reattachment of buffers
- * to a dirty folio will set *all* buffers dirty. Which would corrupt
- * filesystem data on the same device.
- *
- * The same applies to regular filesystem folios: if all the buffers are
- * clean then we set the folio clean and proceed. To do that, we require
- * total exclusion from block_dirty_folio(). That is obtained with
- * i_private_lock.
- *
- * try_to_free_buffers() is non-blocking.
- */
static inline int buffer_busy(struct buffer_head *bh)
{
return atomic_read(&bh->b_count) |
@@ -2917,6 +2897,30 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
return false;
}
+/**
+ * try_to_free_buffers: Release buffers attached to this folio.
+ * @folio: The folio.
+ *
+ * If any buffers are in use (dirty, under writeback, elevated refcount),
+ * no buffers will be freed.
+ *
+ * If the folio is dirty but all the buffers are clean then we need to
+ * be sure to mark the folio clean as well. This is because the folio
+ * may be against a block device, and a later reattachment of buffers
+ * to a dirty folio will set *all* buffers dirty. Which would corrupt
+ * filesystem data on the same device.
+ *
+ * The same applies to regular filesystem folios: if all the buffers are
+ * clean then we set the folio clean and proceed. To do that, we require
+ * total exclusion from block_dirty_folio(). That is obtained with
+ * i_private_lock.
+ *
+ * Exclusion against try_to_free_buffers may be obtained by either
+ * locking the folio or by holding its mapping's i_private_lock.
+ *
+ * Context: Process context. @folio must be locked. Will not sleep.
+ * Return: true if all buffers attached to this folio were freed.
+ */
bool try_to_free_buffers(struct folio *folio)
{
struct address_space * const mapping = folio->mapping;
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers()
2024-01-09 14:33 ` [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers() Matthew Wilcox (Oracle)
@ 2024-01-10 14:21 ` Pankaj Raghav (Samsung)
2024-01-11 3:22 ` Randy Dunlap
1 sibling, 0 replies; 15+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-01-10 14:21 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jonathan Corbet, linux-doc, linux-fsdevel, linux-kernel, p.raghav
On Tue, Jan 09, 2024 at 02:33:52PM +0000, Matthew Wilcox (Oracle) wrote:
> The documentation for this function has become separated from it over
> time; move it to the right place and turn it into kernel-doc. Mild
> editing of the content to make it more about what the function does, and
> less about how it does it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Looks good,
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers()
2024-01-09 14:33 ` [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers() Matthew Wilcox (Oracle)
2024-01-10 14:21 ` Pankaj Raghav (Samsung)
@ 2024-01-11 3:22 ` Randy Dunlap
1 sibling, 0 replies; 15+ messages in thread
From: Randy Dunlap @ 2024-01-11 3:22 UTC (permalink / raw)
To: Matthew Wilcox (Oracle), Jonathan Corbet
Cc: linux-doc, linux-fsdevel, linux-kernel
Hi,
On 1/9/24 06:33, Matthew Wilcox (Oracle) wrote:
> The documentation for this function has become separated from it over
> time; move it to the right place and turn it into kernel-doc. Mild
> editing of the content to make it more about what the function does, and
> less about how it does it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/buffer.c | 44 ++++++++++++++++++++++++--------------------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 071f01b28c90..25861241657f 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2864,26 +2864,6 @@ int sync_dirty_buffer(struct buffer_head *bh)
> }
> EXPORT_SYMBOL(sync_dirty_buffer);
>
> -/*
> - * try_to_free_buffers() checks if all the buffers on this particular folio
> - * are unused, and releases them if so.
> - *
> - * Exclusion against try_to_free_buffers may be obtained by either
> - * locking the folio or by holding its mapping's i_private_lock.
> - *
> - * If the folio is dirty but all the buffers are clean then we need to
> - * be sure to mark the folio clean as well. This is because the folio
> - * may be against a block device, and a later reattachment of buffers
> - * to a dirty folio will set *all* buffers dirty. Which would corrupt
> - * filesystem data on the same device.
> - *
> - * The same applies to regular filesystem folios: if all the buffers are
> - * clean then we set the folio clean and proceed. To do that, we require
> - * total exclusion from block_dirty_folio(). That is obtained with
> - * i_private_lock.
> - *
> - * try_to_free_buffers() is non-blocking.
> - */
> static inline int buffer_busy(struct buffer_head *bh)
> {
> return atomic_read(&bh->b_count) |
> @@ -2917,6 +2897,30 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
> return false;
> }
>
> +/**
> + * try_to_free_buffers: Release buffers attached to this folio.
preferably s/_buffers: /_buffers - /
> + * @folio: The folio.
> + *
> + * If any buffers are in use (dirty, under writeback, elevated refcount),
> + * no buffers will be freed.
> + *
> + * If the folio is dirty but all the buffers are clean then we need to
> + * be sure to mark the folio clean as well. This is because the folio
> + * may be against a block device, and a later reattachment of buffers
> + * to a dirty folio will set *all* buffers dirty. Which would corrupt
> + * filesystem data on the same device.
> + *
> + * The same applies to regular filesystem folios: if all the buffers are
> + * clean then we set the folio clean and proceed. To do that, we require
> + * total exclusion from block_dirty_folio(). That is obtained with
> + * i_private_lock.
> + *
> + * Exclusion against try_to_free_buffers may be obtained by either
> + * locking the folio or by holding its mapping's i_private_lock.
> + *
> + * Context: Process context. @folio must be locked. Will not sleep.
> + * Return: true if all buffers attached to this folio were freed.
> + */
> bool try_to_free_buffers(struct folio *folio)
> {
> struct address_space * const mapping = folio->mapping;
--
#Randy
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/8] buffer: Fix __bread and __bread_gfp kernel-doc
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
` (2 preceding siblings ...)
2024-01-09 14:33 ` [PATCH v2 3/8] buffer: Add kernel-doc for try_to_free_buffers() Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse() Matthew Wilcox (Oracle)
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel,
Pankaj Raghav
The extra indentation confused the kernel-doc parser, so remove it.
Fix some other wording while I'm here, and advise the user they need to
call brelse() on this buffer.
__bread_gfp() isn't used directly by filesystems, but the other wrappers
for it don't have documentation, so document it accordingly.
Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 35 ++++++++++++++++++++++-------------
include/linux/buffer_head.h | 22 +++++++++++++---------
2 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 25861241657f..160bbc1f929c 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1453,20 +1453,29 @@ void __breadahead(struct block_device *bdev, sector_t block, unsigned size)
EXPORT_SYMBOL(__breadahead);
/**
- * __bread_gfp() - reads a specified block and returns the bh
- * @bdev: the block_device to read from
- * @block: number of block
- * @size: size (in bytes) to read
- * @gfp: page allocation flag
- *
- * Reads a specified block, and returns buffer head that contains it.
- * The page cache can be allocated from non-movable area
- * not to prevent page migration if you set gfp to zero.
- * It returns NULL if the block was unreadable.
+ * __bread_gfp() - Read a block.
+ * @bdev: The block device to read from.
+ * @block: Block number in units of block size.
+ * @size: The block size of this device in bytes.
+ * @gfp: Not page allocation flags; see below.
+ *
+ * You are not expected to call this function. You should use one of
+ * sb_bread(), sb_bread_unmovable() or __bread().
+ *
+ * Read a specified block, and return the buffer head that refers to it.
+ * If @gfp is 0, the memory will be allocated using the block device's
+ * default GFP flags. If @gfp is __GFP_MOVABLE, the memory may be
+ * allocated from a movable area. Do not pass in a complete set of
+ * GFP flags.
+ *
+ * The returned buffer head has its refcount increased. The caller should
+ * call brelse() when it has finished with the buffer.
+ *
+ * Context: May sleep waiting for I/O.
+ * Return: NULL if the block was unreadable.
*/
-struct buffer_head *
-__bread_gfp(struct block_device *bdev, sector_t block,
- unsigned size, gfp_t gfp)
+struct buffer_head *__bread_gfp(struct block_device *bdev, sector_t block,
+ unsigned size, gfp_t gfp)
{
struct buffer_head *bh;
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index d78454a4dd1f..56a1e9c1e71e 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -437,17 +437,21 @@ static inline void bh_readahead_batch(int nr, struct buffer_head *bhs[],
}
/**
- * __bread() - reads a specified block and returns the bh
- * @bdev: the block_device to read from
- * @block: number of block
- * @size: size (in bytes) to read
+ * __bread() - Read a block.
+ * @bdev: The block device to read from.
+ * @block: Block number in units of block size.
+ * @size: The block size of this device in bytes.
*
- * Reads a specified block, and returns buffer head that contains it.
- * The page cache is allocated from movable area so that it can be migrated.
- * It returns NULL if the block was unreadable.
+ * Read a specified block, and return the buffer head that refers
+ * to it. The memory is allocated from the movable area so that it can
+ * be migrated. The returned buffer head has its refcount increased.
+ * The caller should call brelse() when it has finished with the buffer.
+ *
+ * Context: May sleep waiting for I/O.
+ * Return: NULL if the block was unreadable.
*/
-static inline struct buffer_head *
-__bread(struct block_device *bdev, sector_t block, unsigned size)
+static inline struct buffer_head *__bread(struct block_device *bdev,
+ sector_t block, unsigned size)
{
return __bread_gfp(bdev, block, size, __GFP_MOVABLE);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse()
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
` (3 preceding siblings ...)
2024-01-09 14:33 ` [PATCH v2 4/8] buffer: Fix __bread and __bread_gfp kernel-doc Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-10 14:30 ` Pankaj Raghav (Samsung)
2024-01-09 14:33 ` [PATCH v2 6/8] buffer: Add kernel-doc for bforget() and __bforget() Matthew Wilcox (Oracle)
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
Move the documentation for __brelse() to brelse(), format it as
kernel-doc and update it from talking about pages to folios.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 17 ++++++++---------
include/linux/buffer_head.h | 16 ++++++++++++++++
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 160bbc1f929c..9a7b3649c872 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1226,17 +1226,16 @@ void mark_buffer_write_io_error(struct buffer_head *bh)
}
EXPORT_SYMBOL(mark_buffer_write_io_error);
-/*
- * Decrement a buffer_head's reference count. If all buffers against a page
- * have zero reference count, are clean and unlocked, and if the page is clean
- * and unlocked then try_to_free_buffers() may strip the buffers from the page
- * in preparation for freeing it (sometimes, rarely, buffers are removed from
- * a page but it ends up not being freed, and buffers may later be reattached).
+/**
+ * __brelse - Release a buffer.
+ * @bh: The buffer to release.
+ *
+ * This variant of brelse() can be called if @bh is guaranteed to not be NULL.
*/
-void __brelse(struct buffer_head * buf)
+void __brelse(struct buffer_head *bh)
{
- if (atomic_read(&buf->b_count)) {
- put_bh(buf);
+ if (atomic_read(&bh->b_count)) {
+ put_bh(bh);
return;
}
WARN(1, KERN_ERR "VFS: brelse: Trying to free free buffer\n");
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 56a1e9c1e71e..3cbc01bbc398 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -303,6 +303,22 @@ static inline void put_bh(struct buffer_head *bh)
atomic_dec(&bh->b_count);
}
+/**
+ * brelse - Release a buffer.
+ * @bh: The buffer to release.
+ *
+ * Decrement a buffer_head's reference count. If @bh is NULL, this
+ * function is a no-op.
+ *
+ * If all buffers on a folio have zero reference count, are clean
+ * and unlocked, and if the folio is clean and unlocked then
+ * try_to_free_buffers() may strip the buffers from the folio in
+ * preparation for freeing it (sometimes, rarely, buffers are removed
+ * from a folio but it ends up not being freed, and buffers may later
+ * be reattached).
+ *
+ * Context: Any context.
+ */
static inline void brelse(struct buffer_head *bh)
{
if (bh)
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse()
2024-01-09 14:33 ` [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse() Matthew Wilcox (Oracle)
@ 2024-01-10 14:30 ` Pankaj Raghav (Samsung)
2024-01-10 17:26 ` Matthew Wilcox
0 siblings, 1 reply; 15+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-01-10 14:30 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jonathan Corbet, linux-doc, linux-fsdevel, linux-kernel, p.raghav
> + * If all buffers on a folio have zero reference count, are clean
> + * and unlocked, and if the folio is clean and unlocked then
IIUC from your [PATCH 3/8], folio only needs to be unlocked to free the
buffers as try_to_free_buffers() will remove the dirty flag and "clean"
the folio?
So:
s/if folio is clean and unlocked/if folio is unlocked
> + * try_to_free_buffers() may strip the buffers from the folio in
> + * preparation for freeing it (sometimes, rarely, buffers are removed
> + * from a folio but it ends up not being freed, and buffers may later
> + * be reattached).
> + *
> + * Context: Any context.
> + */
> static inline void brelse(struct buffer_head *bh)
> {
> if (bh)
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse()
2024-01-10 14:30 ` Pankaj Raghav (Samsung)
@ 2024-01-10 17:26 ` Matthew Wilcox
2024-01-10 20:10 ` Pankaj Raghav (Samsung)
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox @ 2024-01-10 17:26 UTC (permalink / raw)
To: Pankaj Raghav (Samsung)
Cc: Jonathan Corbet, linux-doc, linux-fsdevel, linux-kernel, p.raghav
On Wed, Jan 10, 2024 at 03:30:54PM +0100, Pankaj Raghav (Samsung) wrote:
> > + * If all buffers on a folio have zero reference count, are clean
> > + * and unlocked, and if the folio is clean and unlocked then
>
> IIUC from your [PATCH 3/8], folio only needs to be unlocked to free the
> buffers as try_to_free_buffers() will remove the dirty flag and "clean"
> the folio?
> So:
> s/if folio is clean and unlocked/if folio is unlocked
That's a good point. Perhaps "unlocked and not under writeback"
would be better wording, since that would be true.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse()
2024-01-10 17:26 ` Matthew Wilcox
@ 2024-01-10 20:10 ` Pankaj Raghav (Samsung)
0 siblings, 0 replies; 15+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-01-10 20:10 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jonathan Corbet, linux-doc, linux-fsdevel, linux-kernel, p.raghav
On Wed, Jan 10, 2024 at 05:26:55PM +0000, Matthew Wilcox wrote:
> On Wed, Jan 10, 2024 at 03:30:54PM +0100, Pankaj Raghav (Samsung) wrote:
> > > + * If all buffers on a folio have zero reference count, are clean
> > > + * and unlocked, and if the folio is clean and unlocked then
> >
> > IIUC from your [PATCH 3/8], folio only needs to be unlocked to free the
> > buffers as try_to_free_buffers() will remove the dirty flag and "clean"
> > the folio?
> > So:
> > s/if folio is clean and unlocked/if folio is unlocked
>
> That's a good point. Perhaps "unlocked and not under writeback"
> would be better wording, since that would be true.
Yeah. That sounds good to me!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 6/8] buffer: Add kernel-doc for bforget() and __bforget()
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
` (4 preceding siblings ...)
2024-01-09 14:33 ` [PATCH v2 5/8] buffer: Add kernel-doc for brelse() and __brelse() Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 7/8] buffer: Improve bdev_getblk documentation Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 8/8] doc: Split buffer.rst out of api-summary.rst Matthew Wilcox (Oracle)
7 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
Distinguish these functions from brelse() and __brelse().
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 9 ++++++---
include/linux/buffer_head.h | 10 ++++++++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 9a7b3649c872..05b68eccfdcc 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1242,9 +1242,12 @@ void __brelse(struct buffer_head *bh)
}
EXPORT_SYMBOL(__brelse);
-/*
- * bforget() is like brelse(), except it discards any
- * potentially dirty data.
+/**
+ * __bforget - Discard any dirty data in a buffer.
+ * @bh: The buffer to forget.
+ *
+ * This variant of bforget() can be called if @bh is guaranteed to not
+ * be NULL.
*/
void __bforget(struct buffer_head *bh)
{
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 3cbc01bbc398..9dc5477f13c7 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -325,6 +325,16 @@ static inline void brelse(struct buffer_head *bh)
__brelse(bh);
}
+/**
+ * bforget - Discard any dirty data in a buffer.
+ * @bh: The buffer to forget.
+ *
+ * Call this function instead of brelse() if the data written to a buffer
+ * no longer needs to be written back. It will clear the buffer's dirty
+ * flag so writeback of this buffer will be skipped.
+ *
+ * Context: Any context.
+ */
static inline void bforget(struct buffer_head *bh)
{
if (bh)
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 7/8] buffer: Improve bdev_getblk documentation
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
` (5 preceding siblings ...)
2024-01-09 14:33 ` [PATCH v2 6/8] buffer: Add kernel-doc for bforget() and __bforget() Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
2024-01-09 14:33 ` [PATCH v2 8/8] doc: Split buffer.rst out of api-summary.rst Matthew Wilcox (Oracle)
7 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
Add some more information about the state of the buffer_head returned.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/buffer.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/buffer.c b/fs/buffer.c
index 05b68eccfdcc..562de7e013f7 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1424,6 +1424,11 @@ EXPORT_SYMBOL(__find_get_block);
* @size: The size of buffer_heads for this @bdev.
* @gfp: The memory allocation flags to use.
*
+ * The returned buffer head has its reference count incremented, but is
+ * not locked. The caller should call brelse() when it has finished
+ * with the buffer. The buffer may not be uptodate. If needed, the
+ * caller can bring it uptodate either by reading it or overwriting it.
+ *
* Return: The buffer head, or NULL if memory could not be allocated.
*/
struct buffer_head *bdev_getblk(struct block_device *bdev, sector_t block,
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 8/8] doc: Split buffer.rst out of api-summary.rst
2024-01-09 14:33 [PATCH v2 0/8] Improve buffer head documentation Matthew Wilcox (Oracle)
` (6 preceding siblings ...)
2024-01-09 14:33 ` [PATCH v2 7/8] buffer: Improve bdev_getblk documentation Matthew Wilcox (Oracle)
@ 2024-01-09 14:33 ` Matthew Wilcox (Oracle)
7 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-01-09 14:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Matthew Wilcox (Oracle), linux-doc, linux-fsdevel, linux-kernel
Buffer heads are no longer a generic filesystem API but an optional
filesystem support library. Make the documentation structure reflect
that, and include the fine documentation kept in buffer_head.h.
We could give a better overview of what buffer heads are all about,
but my enthusiasm for documenting it is limited.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
Documentation/filesystems/api-summary.rst | 3 ---
Documentation/filesystems/index.rst | 1 +
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/Documentation/filesystems/api-summary.rst b/Documentation/filesystems/api-summary.rst
index 98db2ea5fa12..cc5cc7f3fbd8 100644
--- a/Documentation/filesystems/api-summary.rst
+++ b/Documentation/filesystems/api-summary.rst
@@ -56,9 +56,6 @@ Other Functions
.. kernel-doc:: fs/namei.c
:export:
-.. kernel-doc:: fs/buffer.c
- :export:
-
.. kernel-doc:: block/bio.c
:export:
diff --git a/Documentation/filesystems/index.rst b/Documentation/filesystems/index.rst
index e18bc5ae3b35..5fc9b19b8d8e 100644
--- a/Documentation/filesystems/index.rst
+++ b/Documentation/filesystems/index.rst
@@ -50,6 +50,7 @@ filesystem implementations.
.. toctree::
:maxdepth: 2
+ buffer
journalling
fscrypt
fsverity
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread