On Tue, Jan 29, 2013 at 12:28:14PM -0500, Theodore Ts'o wrote:
> On Tue, Jan 29, 2013 at 01:34:15PM +0800, Zheng Liu wrote:
> >
> > Another choice is like this:
> >
> > struct extent_status {
> > ...
> > ext4_fsblk_t es_pblk; /* first physical block */
> > };
> >
> > #define EXTENT_STATUS_WRITTEN (1ULL << 60)
> > #define EXTENT_STATUS_UNWRITTEN (1ULL << 61)
> > #define EXTENT_STATUS_DELAYED (1ULL << 62)
> >
> > When we want to set extent status, we will need to do like the following:
> >
> > es->es_pblk |= EXTENT_STATUS_WRITTEN;
> >
> > This can make us avoid non-protable code. I am happy to refine this
> > patch if you think the latter one is better.
>
> This is probably the way I would have done it myself, but the then you
> need to make sure that all of the places where es_pblk is used you
> have to mask off the high bits.
>
> At this point, though, I don't think it's worth it to make the change
> now, especially since we're almost at -rc6, I want to make sure this
> gets into linux-next and so we get lots of testing.
>
> As a matter of fact, I've already started testing the v3 vesion of the
> extent status patches from January 23rd, with the v2 version of the
> slab reclaim patch. It's in the unstable portion of the ext4 git
> tree, at:
>
> http://repo.or.cz/w/ext4-patch-queue.git /
> git://repo.or.cz/ext4-patch-queue.git
>
> I'm waiting for your next version of your patch series before I move
> it into the dev branch which will get fed into linux-next; my
> understanding is you're just about ready to push it out, right?
Yes, I am running xfstests to make sure that the patch series doesn't
break anything. Later it will be sent out.
>
> If we want to move away from using bitfields, we can do that as a
> separate patch that gets submitted later, since that's pretty easy to
> audit and verify for correctness. Also, I've since tested clang and
> noted that it supports bitfields for unsigned long long. There is
> some differences between how gcc and clang handles sign extension for
> unsigned long values, though:
>
> #include <stdio.h>
> struct s
> {
> unsigned long long a:2;
> unsigned long long b:40;
> unsigned long long c:22;
> };
>
> int main()
> {
> struct s t = {1, 2, 3};
> printf("0x%llx\n",(t.b-8));
> }
>
> Gcc 4.7.2 will print "0xfffffffffa", while clang 3.0-6 will print
> "0xfffffffffffffffa" for the same program.
Clang is first coming in my mind. I know that some one try to use it
to build a linux kernel and get a lot of problems that are about gcc
extension. But for us it seems that things are not too bad. ;)
>
> I don't think this is a huge issue for us, but it's worth keeping in
> mind...
>
> So let's go ahead and keep the bitfields at least for the initial
> patch submission.
Yes, just keep in mind and go ahead.
Thanks for teaching me a lot,
- Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
