linux-kernel - Re: [PATCH take2 3/5] dio: formalize bio counters as a dio reference count

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]

Message-ID: <4522F0F2.3030206@oracle.com>
Date:	Tue, 03 Oct 2006 16:23:30 -0700
From:	Zach Brown <zach.brown@...cle.com>
To:	Andrew Morton <akpm@...l.org>
CC:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-aio@...ck.org
Subject: Re: [PATCH take2 3/5] dio: formalize bio counters as a dio reference
 count

>> +static int wait_for_more_bios(struct dio *dio)
>> +{
>> +	assert_spin_locked(&dio->bio_lock);
>> +
>> +	return (atomic_read(&dio->refcount) > 1) && (dio->bio_list == NULL);
>> +}
> 
> This function isn't well-named.

Sure, I'll try harder.

> 
>> @@ -1103,7 +1088,11 @@ direct_io_worker(int rw, struct kiocb *i
>>  		}
>>  		if (ret == 0)
>>  			ret = dio->result;
>> -		finished_one_bio(dio);		/* This can free the dio */
>> +
>> +		/* this can free the dio */
>> +		if (atomic_dec_and_test(&dio->refcount))
>> +			dio_complete_aio(dio);
> 
> So...  how come it's legitimate to touch *dio if it can be freed by now? 

Indeed!  This point in the patchset is retaining the existing behaviour
where the 'should_wait' direct_io_worker() path and the
finished_one_bio() path very carefully keep their tests in sync so that
only one of them ends up freeing the dio.

This particular patch doesn't change this part of the behaviour, it's
just bringing the dec and complete that was previously hidden in
finished_one_bio() up into its caller.  So I kept the comment.

This mess is replaced with a proper

	if (dec_and_test(dio->refcount))
		kfree(dio)

construct in the final patch.  I hope that means we don't need to patch
in some comments that we later remove.

- z
-
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/