> -----Original Message----- > From: Matt Fleming [mailto:matt@xxxxxxxxxxxxxxxxxxx] > Sent: Thursday, January 28, 2016 8:16 PM > > On Tue, 26 Jan, at 03:10:03AM, Kweh Hock Leong wrote: > > > > > > This mutex is not needed. It doesn't protect anything in your code. > > > > This is to synchronize/serializes one of the instant is calling > efi_capsule_supported() > > and the other one is calling efi_capsule_update() which they are exported > symbol > > from capsule.c. > > You don't need to synchronise them. > > Looking at my original capsule patches I can see why you might be > doing this locking, but it's unnecessary. You don't need to serialise > access to efi_capsule_supported() and efi_capsule_update(). > > Internally to the core EFI capsule code we need to ensure that we > don't allow capsules with conflicting reset types to be sent to the > firmware (how would we know the type of reset to perform?), but that > should be entirely transparent to your driver. > > I'm planning on re-sending my capsule patches, so all this should > become clearer. > Ok. Noted. Will remove it and submit for v11. > > > This function would be much more simple if you handled the above > > > condition differently. > > > > > > Instead of having code in efi_capsule_setup_info() to allocate the > > > initial ->pages memory, why not just allocate it in efi_capsule_open() > > > at the same time as you allocate the private_data? You can then > > > free it in efi_capsule_release() (you're leaking it at the moment). > > > > > > You are always going to need at least one element in ->pages for > > > successful operation, so you might as well allocate it up front. > > > > Just want to double check I understand you correctly. Do you mean > > I should free ->pages during close(2) but not free the ->pages[X] if > > there is any successfully submitted? > > Yes, that's correct. Ok. Noted. Thanks & Regards, Wilson -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
