[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/9] parted: fix EOF and ctrl-c handling

From: Phillip Susi
Subject: Re: [PATCH 1/9] parted: fix EOF and ctrl-c handling
Date: Tue, 13 Aug 2013 23:46:44 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7

On 08/08/2013 08:45 PM, Brian C. Lane wrote:
Commit messages need to follow the normal parted form. I'd also like to
see tests as separate commits.

Can you be a bit more specific about the form? As far as I know this is the usual form that Jim had me use. He also stressed that new feature and test for it go hand in hand, and logically they are two parts of the same conceptual change, so it makes sense to keep them in the same commit.

4/9 - Nak. I have a more general patch for this -- not everything
            inserts DMRAID in there.

Could you be a bit more specific?

5/9 - Shouldn't it use disk->dev->sector_size instead of hard-coded
       512/1024? The logic seems a bit messy, using an if block instead
       of the == 1 ? 512 : 1024 may clean it up a little. If it is
       already set to 512 (or really ->sector_size) we don't need to walk
       the other partitions. One place where a tab snuck in. Looks pretty
       ok other than that.

I'm not sure what you mean about walking the other partitions. I suppose it should use ->sector size. I'll have to check the kernel sources again to see how they behave with non 512 byte sectors.

6/9 - Nak. I'm not sure if this is the right way to handle things.

Could you be more specific?

7/9 - Nak. I really don't like the giant pile of changes for a problem
       that is only in the gpt code.

Do you have another suggestion? I don't see another way of fixing the bug, and the constant re-reading of the partition table seems a senseless waste of time to begin with, so it kills two birds with one stone.

8/9 - Nak. It looks to me like removing some of these checks is going to
       allow user (ui or library) checks that should fail on new data to
       pass. I'm ok with letting us run with these errors so we can fix
       them, but you shouldn't be able to enter bad numbers after the

I thought that the ui did its own checks and stops you before it gets to the library but I'll take a look at that again.

9/9 - Nak. I need more details here. The reasoning in the original
       commit seems sound to me, and I'd be more worried about busy
       devices than the filesystem not matching (I'm not sure we should
       even be in the fs detection business anymore).

The reasoning was sound, it was just based on a falsehood: the kernel does indeed still maintain two separate caches for the two devices, so when you write a fs superblock to the partition device then try to read it via the whole disk device, you don't see the superblock unless you flush the caches. See:


I never heard back from Hans about why he put that in, but removing it fixed the problem, and I couldn't find any attempt to resolve the cache aliasing problem in the code, or heard of anyone talking about it on linux-mm.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]