From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758598AbcIMOda (ORCPT ); Tue, 13 Sep 2016 10:33:30 -0400 Received: from mout.web.de ([212.227.15.14]:61174 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758541AbcIMOdZ (ORCPT ); Tue, 13 Sep 2016 10:33:25 -0400 Subject: Re: virtio_blk: Less function calls in init_vq() after error detection To: =?UTF-8?Q?Christian_Borntr=c3=a4ger?= , virtualization@lists.linux-foundation.org, "Michael S. Tsirkin" References: <566ABCD9.1060404@users.sourceforge.net> <02054675-8395-ac81-6863-e3a5cbfc9032@users.sourceforge.net> Cc: Julia Lawall , kernel-janitors@vger.kernel.org, LKML , linux-doc@vger.kernel.org, Minfei Huang From: SF Markus Elfring Message-ID: <55fbd4d0-3fb3-0334-6545-cb4da0d12edf@users.sourceforge.net> Date: Tue, 13 Sep 2016 16:33:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:DcLc07tY2GCEZGnnjyHPgGU6p3+T/Kbut76cMvY0p0iP9IPlSQ/ 0uJaMNWI0ejQ0xbE6nUtFvO966QpXXNf41ypUhHez7Gpasl9Rasl8+R4sHVMZb2ZLz0jue/ UG18KMxjGd1meOfUwX1Uw72V+JU6u6olxnq/kN+dDob+ZtvoakuqrwnorE0nSnYkaYuLw74 ItQjWrU1aGkRONkaYQDMg== X-UI-Out-Filterresults: notjunk:1;V01:K0:7nV3Lkowuak=:H3JWsPvc8qpoBrjrC//zuL RsgNr2uA2JStVdBgAb6sVV8vGC6wshLSW5XijMjD+xl407lb+2/P7CPw2O0+Ye3l1IKOPWC/R W+KY/+wTpqU6EHU87Nycboo0w/4K9tx1RJnI7znFCK8iGgvsJj8D8cKPTIvj2NUci0uPmkRUJ FlKd/DWfVK+eayl6TFrB8oZyJ0AEirGDojC98VNk1JXgwvcs2Ag3ceTztH/Eyt2mopG1RSZEq ZjBAB8EOVgylzLMYW6JhvOdzzYzr2pfQ0oTomj7COYfYxz2bSBtQN4QUwA0y+UYcMTvR82L9t dwZJXvBdm6N7hN7sVlOZZK4Rs0nbYkEaF14Gt7Vntykt/cgVSgLeO2CO4+ryLLHYt+jvh8GMe 22YYW+ZL5TH0YuTW1icFPUg03tnPH2q9TDmYGhMFTmxEeolgasLIkXvrdUNacu7rJAad1k/dX wp3fPgEPrstJ4G5RJLTpD6EDfI+p7sx6cq4a84Zm9kdjEeavh+0HW8W9O7Ocp3wm1+nnQGNTN ZTOcGXIGJm/uT1jyHtFCsjYUrw+RlfqwRcJn2OBwQSvA+Ij5YQzKJCJh7s64BtP5xu8kezE9u 7XTnfMfhJuuAQYgWZF0bEYKfMPotRyY7rUuhQh7nkP/J3Y66CjfjosUZZ0wL1A3AapHML6/oO VLT0NwUIvj3tCsejw/e7anBIT7/8y/wOWPvwkeZAu/QPy3HM/bzGUSXDBaPYI3VjT0qnZidrz Ukqd/um4azzijQJvdhwQEV6cbnqLKTD1q6whClXgI8yaEs5Ym0I5iBqUdwNlN8nW9nUBncHGo zpYFnyt Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> drivers/block/virtio_blk.c | 22 +++++++++++++++++----- >> 1 file changed, 17 insertions(+), 5 deletions(-) > > Can't you see from this diffstat that the patch actually seems to makes > the code more complex? I find that the repeated usage of a bit more error handling code is almost unavoidable if you would like to handle allocation failures more directly as I dared to propose again here. > In addition, please have a look at commit 347a529398e8e723338cca5d8a8ae2d9e7e93448 > virtio_blk: Fix a slient kernel panic > > which did the opposite of your patch. This software update adjusted also the jump targets. This approach triggered another update suggestion (in addition to improvements around the function "kmalloc_array"). Such a software development shows different views on the implementation for correct exception handling. I am not so "silent" on this development topic for years. > And in fact it fixed a bug. I get the impression from Minfei's contribution that the statement "err = -ENOMEM;" was added behind memory allocations. It was also chosen to restructure this function implementation so that the single label "out" was used there for a while. * Is this detail worth for another look? * How does this name selection fit to the current Linux coding style convention? > Quite obviously multiple labels are harder to read I do not agree agree completely to your opinion. > and harder to get right. These identifiers can generate their own kind of software development challenges as usual. > For error handling with just kfree one label is just the right thing to. This approach can look convenient at first glance. Does the correctness aspect need any further considerations? Regards, Markus