From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424020AbcBQPlA (ORCPT ); Wed, 17 Feb 2016 10:41:00 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:50948 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422745AbcBQPk6 (ORCPT ); Wed, 17 Feb 2016 10:40:58 -0500 From: Arnd Bergmann To: Andrzej Hajda Cc: linux-kernel@vger.kernel.org, Bartlomiej Zolnierkiewicz , Marek Szyprowski , viro@zeniv.linux.org.uk Subject: Re: [PATCH v2 1/7] netfilter: fix IS_ERR_VALUE usage Date: Wed, 17 Feb 2016 16:40:50 +0100 Message-ID: <3447722.0lPRP5Bqf7@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <56C48993.1050508@samsung.com> References: <20160217023127.GG17997@ZenIV.linux.org.uk> <10818862.KbfsAs5PD4@wuerfel> <56C48993.1050508@samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:R21y8W3L+mLdfQB2nJBwyHvfCZAjQhXZjvQM3/WzNKNcHaTrkbM LZI8QsipnxLC/oQXEbwXLIzHpBlotwFiyB5rKDKSdGRteEe7XJhofv7zfdiwDm3AcCmU5UY 8GWGeJAVZy7O23sM6Vs0Bt/joTJdE4dgJWSK5sIICqoQFYUq/C7L/l9ltfqOHjG1P1jChnU veMyqak0R+YFGnYyXEsaA== X-UI-Out-Filterresults: notjunk:1;V01:K0:5V+2Ra5GoqE=:tM/X8KWXsGnCBDBAGVCfv4 w4FFswrTMzW4htgKLCvRO2VyniUvR0PbqI9iC9KdocrotPezUhIxvfDq+Ttgi+Kqb5ilWQFqH ZOY0DJBxsGMyKisIATGbc4fIw9JADW49HhKHYZziLINyCcPomac7WZOiY8y2d0lTwyQYJDQru AoHPHoV+5KtobT4xzlLdFFWXVE0kIQdDFfyi7HyIL9z7vsWuWstTqIHnGqywk2nWeEjIpsUQV ERm+TrkndNw4y+2p5jgJ30lz8JUPQqHE2064ACyqofg940BS7QvLqfg6rGlLCSVw6JbE8Axez S4JFmfJQWvok8sdPrYC3DYsHbXsbCqmMEQndVZNnn+Z8gHExs18zipN9QcPiKT474pR+Br7wd GM4XsggJorpDplaEbY9PgUDUxI2eFf3rQGBpwcOyA82zvcCQXB1/66f8N5ZcbTJ5E+sLZw7qY YbMvb35rxFiCrsFrQwoZL5tCoHYpIltvjnngjLPWadkIjxRNjNO6uTpZc70AIRc41t5p/ZnQk T+Pae6oHo6avs1TLArIVb5ZRuRnch2WoNHEzvHneNOorDw0APixftz66aAtWkk9qfizKfcs4q c510GrdKKb/oT/M4wmWpol5XswI6otmBKAUxf133x6GupRf/gkiW1CTdMM1P6W6hGbVUyzH5Y BTGayUV7wGdT63TThxP6rnw2WjLQCC5xvBOK4qxIz/YfOV0YQIkBtXjYhk1fQABWx9X9kM8q4 XQBLQgFSQniEInwC Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 17 February 2016 15:54:11 Andrzej Hajda wrote: > > However, could you put the union into the three users (struct arpt_entry > > etc) to avoid having to cast the inner structure into the union using > > container_of()? It doesn't feel right to use container_of() in this > > way here. > > > > > I am not sure if you are aware of the fact these structures are exposed > to user > space. Is it OK to add such unions to them? > You are right, that would be odd. My first idea was actually to put a union into struct xt_counters, and I did notice that this was exposed to user space so I did not mention it. Putting a union into arpt_entry etc would be worse then. The only alternative I see would be to define xt_counters as struct xt_counters { #ifndef __KERNEL__ __u64 pcnt, bcnt; /* Packet and byte counters */ #else union { __u64 pcnt; struct xt_counters __percpu *xt_smp_counters; }; __u64 bcnt; #endif }; but that is still really ugly, and no real improvement over your approach. One really simple fix would be to basically open-code a correct version of IS_ERR_VALUE specifically for xt_counters and leave everything using the __u64 hack: -static inline u64 xt_percpu_counter_alloc(void) +static inline int xt_percpu_counter_alloc(struct xt_counters *cnt) { if (nr_cpu_ids > 1) { void __percpu *res = __alloc_percpu(sizeof(struct xt_counters), sizeof(struct xt_counters)); if (res == NULL) - return (u64) -ENOMEM; + return -ENOMEM; - return (u64) (__force unsigned long) res; + cnt->pcnt = (u64)(uintptr_t)res; } return 0; } that avoids the union but keeps the implicit overloading of the pcnt field, just local to the alloc/free functions. Arnd