Subject: Re: bug#7362: dd strangeness

From: Pádraig Brady <[email-masked]>
Date: Wed, 02 Mar 2011 23:06:43 +0000

On 02/03/11 17:55, Paul Eggert wrote:
> On 03/02/2011 04:53 AM, Pádraig Brady wrote:
>> +  /* Enable 'fullblock' with 'direct' or 'cio' as we don't want to
>> +     write partial blocks to output, which would disable O_DIRECT. An
>> +     alternative would be to enable C_TWOBUFS to accumulate full output
>> +     blocks.  However that wouldn't work when a count is specified, and
>> +     is also less efficient.  */
>> +  if (output_flags & (O_DIRECT | O_CIO))
>> +    input_flags |= O_FULLBLOCK;
>
> I'm afraid this patch feels wrong somehow.  It's conflating four
> issues: counting, efficiency, disabling O_DIRECT, and disabling O_CIO.
> Some thoughts:
>
> 1.  Offhand I don't see why O_CIO has anything to do with blocking;
> why is it mentioned here?

Well in my quick googling it seemed to be a superset
of O_DIRECT mode, with the same alignment constraints.
It seems to fall back to a standard (synchronous) write
if the alignment is not maintained (we should always be OK there),
so I thought it safer to add it given its close relationship
with O_DIRECT. I'll remove it until I can actually test this.

>
> 2.  If C_TWOBUFS is already in effect, then dd needn't set
> O_FULLBLOCK, since C_TWOBUFS already prevents the disabling of
> O_DIRECT.

True. But it shouldn't cause an issue.
I'll leave as is to simplify the code and docs.
I'll amend the comment.

> 3.  The counting issue is independent of oflag=direct or oflag=cio:
> any solution for counting should work regardless of oflag settings.

True. I'll amend the comment.

> The more I think about it, the more I suspect that this
> O_FULLBLOCK-inferring code should be omitted.  It adds little benefit, as
> it covers a rare combination of flags that is likely to be used only
> by experts, who should know the gotchas in this area anyway.  And the
> extra complexity in documentation will penalize everybody who reads
> it, even the non-experts.

Well it has caught people out recently:
https://bugzilla.redhat.com/show_bug.cgi?id=614605
https://lkml.org/lkml/2011/2/22/746

I'm still inclined to add this.

Thanks for all the insightful comments on this BTW.
dd really is hairy.

Pádraig.