[PATCH V2 0/2] Fix a couple comments

[PATCH V2 0/2] Fix a couple comments

[cem]

From: Carlos Maiolino <cem@kernel.org>

Fix a couple comments in xfs ail code

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Carlos Maiolino (2):
  xfs: Fix a comment on xfs_ail_delete
  xfs: Fix comment on xfs_trans_ail_update_bulk()

 fs/xfs/xfs_trans_ail.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

-- 
2.49.0

Re: [PATCH V2 0/2] Fix a couple comments

[Carlos Maiolino]

On Mon, May 12, 2025 at 01:30:21PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 

Please ignore this patch, git-send-email crashed on the middle and didn't send
everything, I'll submit it again.

Sorry the noise.

> Fix a couple comments in xfs ail code
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> Carlos Maiolino (2):
>   xfs: Fix a comment on xfs_ail_delete
>   xfs: Fix comment on xfs_trans_ail_update_bulk()
> 
>  fs/xfs/xfs_trans_ail.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> --
> 2.49.0
> 
> 

[PATCH V2 0/2] Fix a couple comments

[cem]

From: Carlos Maiolino <cem@kernel.org>

Fix a couple comments in the AIL code

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Carlos Maiolino (2):
  xfs: Fix a comment on xfs_ail_delete
  xfs: Fix comment on xfs_trans_ail_update_bulk()

 fs/xfs/xfs_trans_ail.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

-- 
2.49.0

[PATCH V2 1/2] xfs: Fix a comment on xfs_ail_delete

[cem]

From: Carlos Maiolino <cem@kernel.org>

It doesn't return anything.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:
	V2:	- Update subject to include xfs subsystem

 fs/xfs/xfs_trans_ail.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 85a649fec6ac..7d327a3e5a73 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -315,7 +315,7 @@ xfs_ail_splice(
 }
 
 /*
- * Delete the given item from the AIL.  Return a pointer to the item.
+ * Delete the given item from the AIL.
  */
 static void
 xfs_ail_delete(
-- 
2.49.0

[PATCH V2 2/2] xfs: Fix comment on xfs_trans_ail_update_bulk()

[cem]

From: Carlos Maiolino <cem@kernel.org>

This function doesn't take the AIL lock, but should be called
with AIL lock held. Also (hopefuly) simplify the comment.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:
	V2: - Fix a busted comment
	    - Wrap lines over 80+ columns
	    - Update subject

 fs/xfs/xfs_trans_ail.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 7d327a3e5a73..67c328d23e4a 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -777,26 +777,28 @@ xfs_ail_update_finish(
 }
 
 /*
- * xfs_trans_ail_update - bulk AIL insertion operation.
+ * xfs_trans_ail_update_bulk - bulk AIL insertion operation.
  *
- * @xfs_trans_ail_update takes an array of log items that all need to be
+ * @xfs_trans_ail_update_bulk takes an array of log items that all need to be
  * positioned at the same LSN in the AIL. If an item is not in the AIL, it will
- * be added.  Otherwise, it will be repositioned  by removing it and re-adding
- * it to the AIL. If we move the first item in the AIL, update the log tail to
- * match the new minimum LSN in the AIL.
+ * be added. Otherwise, it will be repositioned by removing it and re-adding
+ * it to the AIL.
  *
- * This function takes the AIL lock once to execute the update operations on
- * all the items in the array, and as such should not be called with the AIL
- * lock held. As a result, once we have the AIL lock, we need to check each log
- * item LSN to confirm it needs to be moved forward in the AIL.
+ * If we move the first item in the AIL, update the log tail to match the new
+ * minimum LSN in the AIL.
  *
- * To optimise the insert operation, we delete all the items from the AIL in
- * the first pass, moving them into a temporary list, then splice the temporary
- * list into the correct position in the AIL. This avoids needing to do an
- * insert operation on every item.
+ * This function should be called with the AIL lock held.
  *
- * This function must be called with the AIL lock held.  The lock is dropped
- * before returning.
+ * To optimise the insert operation, we add all items to a temporary list, then
+ * splice this list into the correct position in the AIL.
+ *
+ * Items that are already in the AIL are first deleted from their current
+ * location before being added to the temporary list.
+ *
+ * This avoids needing to do an insert operation on every item.
+ *
+ * The AIL lock is dropped by xfs_ail_update_finish() before returning to
+ * the caller.
  */
 void
 xfs_trans_ail_update_bulk(
-- 
2.49.0

Re: [PATCH V2 2/2] xfs: Fix comment on xfs_trans_ail_update_bulk()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH V2 0/2] Fix a couple comments

[Chandan Babu R]

On Mon, May 12, 2025 at 01:42:54 PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> Fix a couple comments in the AIL code
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
>
> Carlos Maiolino (2):
>   xfs: Fix a comment on xfs_ail_delete
>   xfs: Fix comment on xfs_trans_ail_update_bulk()
>
>  fs/xfs/xfs_trans_ail.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)

Both the patches look good to me,

Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>

-- 
Chandan

Re: [PATCH V2 0/2] Fix a couple comments

[Carlos Maiolino]

On Mon, 12 May 2025 13:42:54 +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Fix a couple comments in the AIL code
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> Carlos Maiolino (2):
>   xfs: Fix a comment on xfs_ail_delete
>   xfs: Fix comment on xfs_trans_ail_update_bulk()
> 
> [...]

Applied to for-next, thanks!

[1/2] xfs: Fix a comment on xfs_ail_delete
      commit: fa8deae92f473a2ebc0e1c7cfa316f5c083e1880
[2/2] xfs: Fix comment on xfs_trans_ail_update_bulk()
      commit: 08c73a4b2e3cd2ff9db0ecb3c5c0dd7e23edc770

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>