| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BANLkTinop+Try_MfxQ+uX_Njn1HaOtGsdw@mail.gmail.com>
Date: Wed, 18 May 2011 21:48:25 -0700
From: Manish Katiyar <mkatiyar@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: ext4 <linux-ext4@...r.kernel.org>, "Theodore Ts'o" <tytso@....edu>
Subject: Re: [PATCH 1/5] jbd2: Pass extra bool parameter in journal routines
to specify if its ok to fail the journal transaction allocation.
On Wed, May 18, 2011 at 5:44 AM, Jan Kara <jack@...e.cz> wrote:
> On Wed 18-05-11 01:36:48, Manish Katiyar wrote:
>> On Wed, May 18, 2011 at 1:09 AM, Jan Kara <jack@...e.cz> wrote:
>> > On Tue 17-05-11 23:38:05, Manish Katiyar wrote:
>> >> On Mon, May 16, 2011 at 8:51 AM, Jan Kara <jack@...e.cz> wrote:
>> >> > Hello,
>> >> >
>> >> > On Thu 12-05-11 23:37:05, Manish Katiyar wrote:
>> >> >> On Wed, May 11, 2011 at 8:54 AM, Jan Kara <jack@...e.cz> wrote:
>> >> >> > Hi,
>> >> >> >
>> >> >> > sorry I got to your patches with a delay. One general note - please do
>> >> >> > not attach patches. It is enough to have them in the email...
>> >> >> >
>> >> >> > On Sun 24-04-11 17:10:41, Manish Katiyar wrote:
>> >> >> >> Pass extra bool parameter in journal routines to specify if its ok to
>> >> >> >> fail the journal transaction allocation. If 'true' is passed
>> >> >> >> transaction allocation is done through GFP_KERNEL and ENOMEM is
>> >> >> >> returned else GFP_NOFS is used.
>> >> >> > Please, do not mix error handling with gfp masks. Instead just rename
>> >> >> > jbd2__journal_start() to jbd2_journal_start() and change gfp_mask parameter
>> >> >> > to "bool errok".
>> >> >>
>> >> >> ok.
>> >> >>
>> >> >> > Use GFP_NOFS gfp mask for start_this_handle().
>> >> >> I think I didn't completely understand this line. You meant passing
>> >> >> GFP_KERNEL or GFP_NOFS based on errok right ?
>> >> > No, I meant passing GFP_NOFS always. Currently, GFP_NOFS is used in all
>> >> > the cases (noone uses GFP_KERNEL variant) and GFP_KERNEL can really be used
>> >> > only when we do not hold other filesystem locks (as GFP_KERNEL allocation
>> >> > can recurse back into filesystem to reclaim memory). So using GFP_KERNEL
>> >> > would need more auditting and is a separate issue anyway.
>> >>
>> >> Hi Jan,
>> >>
>> >> How about this ? As suggested I have removed special handlers for
>> >> ocfs2, always pass false as default for allocation and have removed
>> >> jbd2__journal_start and collapsed it in jbd2_journal_start.
>> >>
>> >>
>> >> Pass extra bool parameter in journal routines to specify if its ok to
>> >> fail the journal transaction allocation. Update ocfs2 and ext4 routines to
>> >> pass false for the updated journal interface.
>> > Yes, now the patch looks good! Thanks. Just one nit below:
>>
>> Thanks a lot. Fixed the newline issue. Here is the updated diff.
>>
>> Pass extra bool parameter in journal routines to specify if its ok to
>> fail the journal transaction allocation. Update ocfs2 and ext4 routines to
>> pass false for the updated journal interface.
> The patch look good now. You can add:
> Acked-by: Jan Kara <jack@...e.cz>
>
> Honza
Signed-off-by: Manish Katiyar <mkatiyar@...il.com>
Acked-by: Jan Kara <jack@...e.cz>
---
fs/ext4/ext4_jbd2.h | 2 +-
fs/ext4/super.c | 2 +-
fs/jbd2/transaction.c | 24 +++++-------------------
fs/ocfs2/journal.c | 6 +++---
include/linux/jbd2.h | 6 ++----
5 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index d0f5353..0abee1b 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -225,7 +225,7 @@ static inline int ext4_journal_extend(handle_t
*handle, int nblocks)
static inline int ext4_journal_restart(handle_t *handle, int nblocks)
{
if (ext4_handle_valid(handle))
- return jbd2_journal_restart(handle, nblocks);
+ return jbd2_journal_restart(handle, nblocks, false);
return 0;
}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8553dfb..4e4c17f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -279,7 +279,7 @@ handle_t *ext4_journal_start_sb(struct super_block
*sb, int nblocks)
ext4_abort(sb, "Detected aborted journal");
return ERR_PTR(-EROFS);
}
- return jbd2_journal_start(journal, nblocks);
+ return jbd2_journal_start(journal, nblocks, false);
}
/*
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 05fa77a..b5c2550 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -318,7 +318,7 @@ static handle_t *new_handle(int nblocks)
*
* Return a pointer to a newly allocated handle, or NULL on failure
*/
-handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int gfp_mask)
+handle_t *jbd2_journal_start(journal_t *journal, int nblocks, bool errok)
{
handle_t *handle = journal_current_handle();
int err;
@@ -338,7 +338,7 @@ handle_t *jbd2__journal_start(journal_t *journal,
int nblocks, int gfp_mask)
current->journal_info = handle;
- err = start_this_handle(journal, handle, gfp_mask);
+ err = start_this_handle(journal, handle, GFP_NOFS);
if (err < 0) {
jbd2_free_handle(handle);
current->journal_info = NULL;
@@ -346,13 +346,6 @@ handle_t *jbd2__journal_start(journal_t *journal,
int nblocks, int gfp_mask)
}
return handle;
}
-EXPORT_SYMBOL(jbd2__journal_start);
-
-
-handle_t *jbd2_journal_start(journal_t *journal, int nblocks)
-{
- return jbd2__journal_start(journal, nblocks, GFP_NOFS);
-}
EXPORT_SYMBOL(jbd2_journal_start);
@@ -441,7 +434,7 @@ out:
* transaction capabable of guaranteeing the requested number of
* credits.
*/
-int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask)
+int jbd2_journal_restart(handle_t *handle, int nblocks, bool errok)
{
transaction_t *transaction = handle->h_transaction;
journal_t *journal = transaction->t_journal;
@@ -477,16 +470,9 @@ int jbd2__journal_restart(handle_t *handle, int
nblocks, int gfp_mask)
lock_map_release(&handle->h_lockdep_map);
handle->h_buffer_credits = nblocks;
- ret = start_this_handle(journal, handle, gfp_mask);
+ ret = start_this_handle(journal, handle, GFP_NOFS);
return ret;
}
-EXPORT_SYMBOL(jbd2__journal_restart);
-
-
-int jbd2_journal_restart(handle_t *handle, int nblocks)
-{
- return jbd2__journal_restart(handle, nblocks, GFP_NOFS);
-}
EXPORT_SYMBOL(jbd2_journal_restart);
/**
@@ -1436,7 +1422,7 @@ int jbd2_journal_force_commit(journal_t *journal)
handle_t *handle;
int ret;
- handle = jbd2_journal_start(journal, 1);
+ handle = jbd2_journal_start(journal, 1, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
} else {
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index b141a44..cca0f76 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -353,11 +353,11 @@ handle_t *ocfs2_start_trans(struct ocfs2_super
*osb, int max_buffs)
/* Nested transaction? Just return the handle... */
if (journal_current_handle())
- return jbd2_journal_start(journal, max_buffs);
+ return jbd2_journal_start(journal, max_buffs, false);
down_read(&osb->journal->j_trans_barrier);
- handle = jbd2_journal_start(journal, max_buffs);
+ handle = jbd2_journal_start(journal, max_buffs, false);
if (IS_ERR(handle)) {
up_read(&osb->journal->j_trans_barrier);
@@ -438,7 +438,7 @@ int ocfs2_extend_trans(handle_t *handle, int nblocks)
if (status > 0) {
trace_ocfs2_extend_trans_restart(old_nblocks + nblocks);
status = jbd2_journal_restart(handle,
- old_nblocks + nblocks);
+ old_nblocks + nblocks, false);
if (status < 0) {
mlog_errno(status);
goto bail;
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index a32dcae..67f0f4f 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1103,10 +1103,8 @@ static inline handle_t *journal_current_handle(void)
* Register buffer modifications against the current transaction.
*/
-extern handle_t *jbd2_journal_start(journal_t *, int nblocks);
-extern handle_t *jbd2__journal_start(journal_t *, int nblocks, int gfp_mask);
-extern int jbd2_journal_restart(handle_t *, int nblocks);
-extern int jbd2__journal_restart(handle_t *, int nblocks, int gfp_mask);
+extern handle_t *jbd2_journal_start(journal_t *, int nblocks, bool errok);
+extern int jbd2_journal_restart(handle_t *, int nblocks, bool errok);
extern int jbd2_journal_extend (handle_t *, int nblocks);
extern int jbd2_journal_get_write_access(handle_t *, struct buffer_head *);
extern int jbd2_journal_get_create_access (handle_t *, struct buffer_head *);
--
1.7.4.1
--
Thanks -
Manish
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists