GParted device refresh speed

Hi Mike,

This is certainly another area that could benefit from some attention.  See:
Bug 631959 - Gparted wasting LOTS of time REPEATEDLY searching partitions after performing function

Perhaps this extra second is from the work around in GParted_Core::commit_to_os when compiled with libparted < 2.2.0?

Tracking when the data becomes "dirty" sounds like a good approach to me.  There are some gotcha's we will have to remember, such as when a logical partition is mounted/unmounted because it might change the status of the extended partition active/inactive.

I agree with keeping this a full scan for the reasons outlined.

With these last two points it might make sense to have a file system object, or perhaps a more general partition contents object to handle more than just file systems (e.g., LVM).  A clean/dirty flag could then be used so that the object would reload the relevant information when needed.
Alternatively we might be able to modify the Partition object to handle this too. 

I think the caching and dirty status tracking of this information is likely one of the best ways to improve scan performance.

Another way I have seen with other tools is to only scan the relevant partition and file system information when the user clicks on the device in the GUI.  That way all devices are not scanned at start-up.  This would spread out the scan time for persons with more than one device, but would not likely be of much benefit to a majority of users with a single disk device.

Your understanding is the same as mine.

Recently I noticed that deleting (clearing) a FAT16/32 volume label is not reflected in the blkid cache.  See Bug 684403 - GParted appends 'nA' to label when changing label.

I wonder if there is a way to indicate that items such as volume labels or UUIDs have changed when there has been no corresponding partition boundary change?

Thank you for the explanation.  Perhaps there is a command or ioctl that could be called in GParted to indicate that the volume label or UUID changed in FAT16/32 file system?

GParted previously used "blkid -c /dev/null".  Unfortunately blkid appears to have problems when the BIOS is misconfigured to indicate a floppy device is present when there is no physical floppy drive installed.  The problem is that blkid hangs for several minutes.  To work around this we changed blkid to use cached results.

The relevant bug report is:
Bug 667511 - Gparted does not start and continues to scan devices

It looks like this change has the undesirable effect of not reflecting volume label and UUID updates to FAT16/32 file systems.  ;-(

EDIT:
This problem does not appear to be fully resolved yet.
See Ubuntu Launchpad Bug 1029149 - gparted automatically starts scanning all devices slowly and never completing the task

Perhaps we need to consider using "blkid -p /path-to-partition" for each partition?
My concern would be that this would add several more seconds to the device refresh time.

EDIT2:
The original intention of using blkid was to be able to capture volume label and UUID information for file systems that do not have a command line equivalent for retrieving this information.  To speed things up, GParted used to rely on the blkid information only, providing that it was not empty.  If empty GParted would try the file system specific command.  This did speed refresh up considerably.  The reason blkid was changed to not be the default was due to UTF-8 language encoding in volume labels.  See Bug 662537 - Ext4 unicode labels not shown correctly.

Good catch Mike.  If blkid did work correctly, then I think GParted should also work correctly.  Having said that GParted does contain code that works around other tool deficiencies.

I noticed that you reported the blkid blank label handling problem on the util-linux mailing list.
bug: blkid doesn't notice when fs labels change to blank

Thanks Mike for looking in to this issue.  And I agree that we should not hold up the 0.14.0 release since this bug is minor.

Thanks for raising the bug report Mike.   Though this is a minor issue in that no data is lost, this issue is definitely one that would be nice to have fixed.

Unfortunately while blkid doesn't recognise a label being changed to blank, using blkid first to read label will give GParted the same issue, undoing anything fixed by Bug 685656 - GParted doesn't notice when file system label is changed to blank.

At the moment i'm slowly going through and understanding how GParted loads all the device, partition and file system information and why it does it the way it does so I can decide how to change it.

Back to refresh speed ...

Finally worked out what the purpose of the code is that uses all the time identified from the timings in comment #3 above.  The calling code is:

src/GParted_Core.cc - void GParted_Core::set_devices( std::vector<Device> & devices )
   297                                  if ( temp_device .highest_busy )
   298                                  {
   299                                          temp_device .readonly = ! commit_to_os( lp_disk, 1 ) ;
   300                                          //Clear libparted messages.  Typically these are:
   301                                          //  The kernel was unable to re-read the partition table...
   302                                          libparted_messages .clear() ;
   303                                  }

The call chain look like this:

commit_to_os(timeout=1)
    ped_disk_commit_to_os()    // takes ~ 1 second
    settle_device(timeout=1)
        Utils::execute_command("udevadm settle --timeout=1")    // takes up to 1 second first time and typically milliseconds thereafter

During every refresh cycle, for every disk the above code is executed.  It is performed to determine if the kernel can be informed of on disk partition changes when there are other partitions on the disk which are active; especially given the comment on line 301.  Example error from fdisk when the kernel can't be informed:

If GParted determines that the kernel can't be informed of partition changes it disables some operations which it considers dangerous.  Those operations appear to be: resize/move partition, copy paste partition into empty space and creating a new partition other than unformatted (that last one seems a bit strange).

Originally the linux kernel could only re-read the whole of the partition table using the BLKRRPART ioctl.  (That's all fdisk and sfdisk still issue now).  This fails with EBUSY (Device or resource busy) when any partition is active.  Newer kernels and tools can also use BLKPG_ADD_PARTITION and BLKPG_DEL_PARTITION ioctls inform the kernel of changes to non-active partitions.  (Guess this includes at least linux >= 2.6 and (lib)parted >= 2.0).

Useful answer in parted-devel email thread of yours
    Warning message if create partition when a partition is mounted

I don't see any reason why we can't get rid of this per device readonly flag and have a single program wide setting which is only ever tested once when scanning the first device for the first time.

(This is the first of several ideas I have for how to address refresh speed).

EDIT: Unfortunately it's not that simple from a program point of view.  Active partitions are under the control of the users.  There may be no active partitions when GParted is started.  The user may then make a partition active and because they are using older software it may not be possible to inform the kernel of the partition changes.  Need to check out exactly which versions of the kernel and libparted are affected and decide if we need to support them or the code needs to be more subtle.

I looked through all the links and there seemed to have been these two separate bugs with GParted / (lib)parted failing to informing the kernel of partition changes after they were applied on the disk.  Both were fixed by applying patches to (lib)parted and incorporating those into GParted live image.  Were there any fixes applied to GParted app?  Can you point them out to me?

Bug reports:

1. Bug 601574 - ERROR: Current NTFS volume size is bigger than the device size!

2. Bug 604298 - Problems resizing file systems with gparted-live-0.5.0-3

Commits (parted upstream URLs):

1. parted: avoid unnecessary open/close on commit, and thus udev activity

2. linux: add wait time and retries to kernel partition reread

When GParted applies each partition add, partition remove or partition resize operation it calls commit_to_os() to inform the kernel of the change, which I will not be changing.  I don't think the speed of applying this is a problem.

However during refresh of the display after all the operations have been applied it calls commit_to_os() again in set_devices() and takes 1 to 2 seconds for each disk which has at least one active partition to discover if GParted will be able to inform the kernel of partition changes in the future.  If not GParted disables some functionality: resize/move partition, copy paste partition into empty space and creating a new partition other than unformatted.  This is what I want to address.

Assuming these extra calls to commit_to_os() during the refresh don't make it work, they are independant and can be reworked.