On Sat, 2008-07-26 at 00:22 -0400, Martin K. Petersen wrote:
> >>>>> "Harvey" == Harvey Harrison <harvey.harrison@xxxxxxxxx> writes:
>
> [Sorry about the delayed response. I've been traveling for a few
> days.]
>
> Harvey> This code should be looked at carefully, while this patch
> Harvey> doesn't change any behaviour, the handling of app_tag, ref_tag
> Harvey> seems odd. The struct defines both as __be16/32 but in these
> Harvey> locations it is read in/written out as cpu-byteorder.
>
> The opaque tag space (app in Type 1+2, app + ref in Type 3) is
> provided for use by filesystems. The SCSI layer doesn't know anything
> about the contents of the tag buffer. It has no idea whether it
> contains bytes, shorts, ints or longs. So we can't swab on the
> assumption that each DIF tuple contains an u16. Just like we don't
> byteswap any other data coming down from above.
>
> Filesystems must prepare tags in an endianness-aware fashion just like
> they prepare normal filesystem metadata. And at the SCSI layer we
> treat it as a byte stream.
OK. But everywhere in this file app_tag is treated as host-order,
and causes no end of sparse warnings.
Looking just at type1 (with some comments):
static void sd_dif_type1_set_tag(void *prot, void *tag_buf, unsigned int sectors)
{
struct sd_dif_tuple *sdt = prot;
char *tag = tag_buf;
unsigned int i, j;
for (i = 0, j = 0 ; i < sectors ; i++, j += 2, sdt++) {
//app_tag is host-order, read in as raw byte in be-order
sdt->app_tag = tag[j] << 8 | tag[j+1];
BUG_ON(sdt->app_tag == 0xffff);
}
}
static void sd_dif_type1_get_tag(void *prot, void *tag_buf, unsigned int sectors)
{
struct sd_dif_tuple *sdt = prot;
char *tag = tag_buf;
unsigned int i, j;
for (i = 0, j = 0 ; i < sectors ; i++, j += 2, sdt++) {
//app_tag is host order again, read out in be-byteorder
tag[j] = (sdt->app_tag & 0xff00) >> 8;
tag[j+1] = sdt->app_tag & 0xff;
}
}
So who cares what the spec says, the code is treating it as a host-order
u16 everywhere. Similarly for the ref_tag. So if you're going to
manipulate them in host-order, just declare the struct in host order.
Anyways, maybe I'm mistaken here.
Harvey
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
