All of lore.kernel.org
 help / color / mirror / Atom feed
From: mzoran@crowfest.net (Michael Zoran)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] staging: vc04_services: rework ioctl code path
Date: Tue, 15 Nov 2016 13:27:14 -0800	[thread overview]
Message-ID: <1479245234.28223.1.camel@crowfest.net> (raw)
In-Reply-To: <20161115095520.GA18059@kroah.com>

On Tue, 2016-11-15 at 10:55 +0100, Greg KH wrote:
> On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> > VCHIQ/vc04_services has a userland device interface
> > that includes ioctls. The current ioctl implementation
> > is a single monolithic function over 1,000+ lines
> > that handles 17 different ioctls through a complex
> > maze of switch and if statements.
> > 
> > The change reimplements that code path by breaking
> > up the code into smaller, easier to maintain functions
> > and uses a dispatch table to invoke the correct function.
> > 
> > Testing:
> > 
> > 1. vchiq_test -f 10 and vchiq_test -p 1 were run from a native
> > 64-bit OS(debian sid).
> > 
> > 2. vchiq_test -f 10 and vchiq_test -p 1 where run from a 32-bit
> > chroot install from the same OS.
> > 
> > Both test cases pass.
> > 
> > This is V2 of this patch.??Changes include:
> > 
> > 1. More code has been moved to the dispatch routine.
> > The dispatch routine is now responsible for copying the top-level
> > data into and out of kernel space by using the data encoded in
> > the ioctl command number.
> > 
> > 2. The number of parameters have been reduced for the handling
> > functions by giving a different prototype to ioctls that pass
> > no arguments.
> > 
> > 3. Macros in linux/compat.h are now used for compatibility data
> > structures.
> > 
> > Signed-off-by: Michael Zoran <mzoran@crowfest.net>
> > ---
> > ?.../vc04_services/interface/vchiq_arm/vchiq_arm.c??| 1733
> > +++++++++++++-------
> > ?.../interface/vchiq_arm/vchiq_ioctl.h??????????????|???96 ++
> > ?2 files changed, 1200 insertions(+), 629 deletions(-)
> 
> This is a rough patch to review, can you break this up into a patch
> series that moves each ioctl to a separate function as needed?
> 
> thanks,
> 
> greg k-h

I'll see what I can do, but since this is getting closer to the
holidays I'm planing to spend less time on this. Perhaps someone else
may be interested in cleaning up the ioctl path?

Since the old code uses obsolete kernel features, it's going to take
some work to fix without adding any checkpatch.pl warnings.

  reply	other threads:[~2016-11-15 21:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-11  6:15 [PATCH v2] staging: vc04_services: rework ioctl code path Michael Zoran
2016-11-12 17:45 ` Stefan Wahren
2016-11-14  9:48 ` Dan Carpenter
2016-11-14 18:28   ` Michael Zoran
2016-11-15  9:55 ` Greg KH
2016-11-15 21:27   ` Michael Zoran [this message]
2016-11-16  6:53     ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1479245234.28223.1.camel@crowfest.net \
    --to=mzoran@crowfest.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.