linux-kernel - Re: What am I doing wrong? submit

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 21 Oct 2010 20:07:59 +0200
From:	Jens Axboe <axboe@...nel.dk>
To:	Boaz Harrosh <bharrosh@...asas.com>
CC:	Ted Ts'o <tytso@....edu>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: What am I doing wrong?  submit_bio() suddenly stops working...

On 2010-10-21 19:41, Boaz Harrosh wrote:
>> +void ext4_io_submit(struct ext4_io_submit *io)
>> +{
>> +	if (!io->io_bio)
>> +		return;
>> +#ifdef PDEBUG
>> +	trace_printk("%s: io submitted io_end %p\n", 
>> +		     io->io_end->inode->i_sb->s_id, io->io_end);
>> +#endif
>> +	submit_bio(io->io_op, io->io_bio);
>> +	ASSERT(!bio_flagged(bio, BIO_EOPNOTSUPP));
>> +	bio_put(io->io_bio);
> 
> The extra get/put is only done for the duration of the ASSERT above, right?
> I'd put a comment. And why don't you just call the _get here just before
> submit_bio instead of down at io_submit_init.

Ted is using it to signal the reference from io:

>> +	io->io_bio = 0;

which makes sense (even if you don't need it, as long as you don't deref
bio after calling submit_bio()).

>> +	do {
>> +		bio = bio_alloc(GFP_NOIO, nvecs);
>> +		nvecs >>= 1;
>> +	} while (bio == NULL);
> 
> This is surly bad. bio_alloc must be allowed to fail
> (Specially with GFP_NOIO). You should only loop down to
> 1 and then prepare to return -ENOMEM from this function
> and handle it properly in callers. (Or schedule and wait
> like below)

Since __GFP_WAIT is set, it'll never return NULL. And as long as you
don't allocate more than 1 before doing you submit_bio(), it should be
OK in this case.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/