jbd2: Fix comment describing journal_init_common()

[PATCH] jbd2: Fix comment describing journal_init_common()

Posted by Daniel Martín Gómez 2 weeks, 2 days ago

The code indicates that journal_init_common() fills the journal_t object
it returns while the comment incorrectly states that only a few fields are
initialised. Also, the comment claims that journal structures could be
created from scratch which isn't possible as journal_init_common() calls
journal_load_superblock() which loads and checks journal superblock from
disk.

Signed-off-by: Daniel Martín Gómez <dalme@riseup.net>
---
 fs/jbd2/journal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 97f487c3d8fc..3c1d42133687 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1518,9 +1518,10 @@ static int journal_load_superblock(journal_t *journal)
  * destroy journal_t structures, and to initialise and read existing
  * journal blocks from disk.  */
 
-/* First: create and setup a journal_t object in memory.  We initialise
- * very few fields yet: that has to wait until we have created the
- * journal structures from from scratch, or loaded them from disk. */
+/* The journal_init_common() function creates and fills a journal_t object
+ * in memory. It calls journal_load_superblock() to load the on-disk journal
+ * superblock and initialize the journal_t object.
+ */
 
 static journal_t *journal_init_common(struct block_device *bdev,
 			struct block_device *fs_dev,
-- 
2.39.5

Re: [PATCH] jbd2: Fix comment describing journal_init_common()

Posted by Theodore Ts'o 1 week, 2 days ago

On Thu, 07 Nov 2024 15:45:38 +0100, Daniel Martín Gómez wrote:
> The code indicates that journal_init_common() fills the journal_t object
> it returns while the comment incorrectly states that only a few fields are
> initialised. Also, the comment claims that journal structures could be
> created from scratch which isn't possible as journal_init_common() calls
> journal_load_superblock() which loads and checks journal superblock from
> disk.
> 
> [...]

Applied, thanks!

[1/1] jbd2: Fix comment describing journal_init_common()
      commit: 3e7c69cdb053f9edea95502853f35952ab6cbf06

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

Re: [PATCH] jbd2: Fix comment describing journal_init_common()

Posted by Zhang Yi 2 weeks ago

On 2024/11/7 22:45, Daniel Martín Gómez wrote:
> The code indicates that journal_init_common() fills the journal_t object
> it returns while the comment incorrectly states that only a few fields are
> initialised. Also, the comment claims that journal structures could be
> created from scratch which isn't possible as journal_init_common() calls
> journal_load_superblock() which loads and checks journal superblock from
> disk.
> 
> Signed-off-by: Daniel Martín Gómez <dalme@riseup.net>

Ha, thank you for the fix. This comment became stale after commit
c30713084ba5 ("jbd2:move load_superblock() into journal_init_common()") in
the "ext4,jbd2: cleanup journal load and initialization process" series.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/journal.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 97f487c3d8fc..3c1d42133687 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1518,9 +1518,10 @@ static int journal_load_superblock(journal_t *journal)
>   * destroy journal_t structures, and to initialise and read existing
>   * journal blocks from disk.  */
>  
> -/* First: create and setup a journal_t object in memory.  We initialise
> - * very few fields yet: that has to wait until we have created the
> - * journal structures from from scratch, or loaded them from disk. */
> +/* The journal_init_common() function creates and fills a journal_t object
> + * in memory. It calls journal_load_superblock() to load the on-disk journal
> + * superblock and initialize the journal_t object.
> + */
>  
>  static journal_t *journal_init_common(struct block_device *bdev,
>  			struct block_device *fs_dev,

Re: [PATCH] jbd2: Fix comment describing journal_init_common()

Posted by Jan Kara 2 weeks, 1 day ago

On Thu 07-11-24 15:45:38, Daniel Martín Gómez wrote:
> The code indicates that journal_init_common() fills the journal_t object
> it returns while the comment incorrectly states that only a few fields are
> initialised. Also, the comment claims that journal structures could be
> created from scratch which isn't possible as journal_init_common() calls
> journal_load_superblock() which loads and checks journal superblock from
> disk.
> 
> Signed-off-by: Daniel Martín Gómez <dalme@riseup.net>

Indeed. Thanks for fixing this up. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/journal.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 97f487c3d8fc..3c1d42133687 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1518,9 +1518,10 @@ static int journal_load_superblock(journal_t *journal)
>   * destroy journal_t structures, and to initialise and read existing
>   * journal blocks from disk.  */
>  
> -/* First: create and setup a journal_t object in memory.  We initialise
> - * very few fields yet: that has to wait until we have created the
> - * journal structures from from scratch, or loaded them from disk. */
> +/* The journal_init_common() function creates and fills a journal_t object
> + * in memory. It calls journal_load_superblock() to load the on-disk journal
> + * superblock and initialize the journal_t object.
> + */
>  
>  static journal_t *journal_init_common(struct block_device *bdev,
>  			struct block_device *fs_dev,
> -- 
> 2.39.5
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR