Date: Wed, 30 Aug 2006 15:53:50 -0700 (PDT)
From: Linus Torvalds <torvalds@osdl.org>
To: Andrew Morton <akpm@osdl.org>
cc: Dave Airlie <airlied@linux.ie>, linux-kernel@vger.kernel.org
Subject: Re: [FOR 2.6.18 FIX][PATCH]  drm: radeon flush TCL VAP for vertex
 program enable/disable
In-Reply-To: <20060830154152.9ac71753.akpm@osdl.org>
Message-ID: <Pine.LNX.4.64.0608301550090.27779@g5.osdl.org>
References: <Pine.LNX.4.64.0608302314360.21600@skynet.skynet.ie>
 <20060830154152.9ac71753.akpm@osdl.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1208
Lines: 29


On Wed, 30 Aug 2006, Andrew Morton wrote:
> 
> That's a somewhat weird-looking patch.  It adds code which is quite
> dissimilar from all the other cases in that switch statement.

It looks ok to me, although you have to look into the caller to see why it 
does what it does.

It would be "prettier" if it changed the size and data of the incoming 
packet instead, but the code as is isn't actually set up to be able to do 
that (the size setup and verification stuff is done before the fixup).

That said, I'd have expected that the VAP state flush is really something 
that the _client_ should do when it generates the commands, not the kernel 
after the fact. Although maybe the kernel could keep track of whether the 
flush is needed at all, and since we apparently allow untrusted generation 
of packets, maybe this is the right approach..

Anyway, the patch doesn't look _wrong_ to me, although it does seem to 
break the abstraction rules a bit.

			Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/