* Re: [Qemu-devel] [PATCH 1/3] Introduce threadlets
From: Venkateswararao Jujjuri (JV) @ 2010-10-20 3:19 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Arun R Bharadwaj, qemu-devel, balbir
In-Reply-To: <4CBE0F5F.3020204@codemonkey.ws>
On 10/19/2010 2:36 PM, Anthony Liguori wrote:
> On 10/19/2010 01:36 PM, Balbir Singh wrote:
>>> + qemu_mutex_lock(&(queue->lock));
>>> + while (1) {
>>> + ThreadletWork *work;
>>> + int ret = 0;
>>> +
>>> + while (QTAILQ_EMPTY(&(queue->request_list))&&
>>> + (ret != ETIMEDOUT)) {
>>> + ret = qemu_cond_timedwait(&(queue->cond),
>>> + &(queue->lock), 10*100000);
>>>
>> Ewww... what is 10*100000, can we use something more meaningful
>> please?
>>
>
> A define is fine but honestly, it's pretty darn obvious what it means...
>
>>> + }
>>> +
>>> + assert(queue->idle_threads != 0);
>>>
>> This assertion holds because we believe one of the idle_threads
>> actually did the dequeuing, right?
>>
>
> An idle thread is a thread is one that is not doing work. At this point in the
> code, we are not doing any work (yet) so if idle_threads count is zero,
> something is horribly wrong. We're also going to unconditionally decrement in
> the future code path which means that if idle_threads is 0, it's going to become
> -1.
>
> The use of idle_thread is to detect whether it's necessary to spawn an
> additional thread.
>
>>> + if (QTAILQ_EMPTY(&(queue->request_list))) {
>>> + if (queue->cur_threads> queue->min_threads) {
>>> + /* We retain the minimum number of threads */
>>> + break;
>>> + }
>>> + } else {
>>> + work = QTAILQ_FIRST(&(queue->request_list));
>>> + QTAILQ_REMOVE(&(queue->request_list), work, node);
>>> +
>>> + queue->idle_threads--;
>>> + qemu_mutex_unlock(&(queue->lock));
>>> +
>>> + /* execute the work function */
>>> + work->func(work);
>>> +
>>> + qemu_mutex_lock(&(queue->lock));
>>> + queue->idle_threads++;
>>> + }
>>> + }
>>> +
>>> + queue->idle_threads--;
>>> + queue->cur_threads--;
>>> + qemu_mutex_unlock(&(queue->lock));
>>> +
>>> + return NULL;
>>>
>> Does anybody do a join on the exiting thread from the pool?
>>
>
> No. The thread is created in a detached state.
>
>>> +}
>>> +
>>> +static void spawn_threadlet(ThreadletQueue *queue)
>>> +{
>>> + QemuThread thread;
>>> +
>>> + queue->cur_threads++;
>>> + queue->idle_threads++;
>>> +
>>> + qemu_thread_create(&thread, threadlet_worker, queue);
>>>
>>
>>> +}
>>> +
>>> +/**
>>> + * submit_threadletwork_to_queue: Submit a new task to a private queue to be
>>> + * executed asynchronously.
>>> + * @queue: Per-subsystem private queue to which the new task needs
>>> + * to be submitted.
>>> + * @work: Contains information about the task that needs to be submitted.
>>> + */
>>> +void submit_threadletwork_to_queue(ThreadletQueue *queue, ThreadletWork *work)
>>> +{
>>> + qemu_mutex_lock(&(queue->lock));
>>> + if (queue->idle_threads == 0&& queue->cur_threads< queue->max_threads) {
>>> + spawn_threadlet(queue);
>>>
>> So we hold queue->lock, spawn the thread, the spawned thread tries to
>> acquire queue->lock
>>
>
> Yup.
>
>>> + }
>>> + QTAILQ_INSERT_TAIL(&(queue->request_list), work, node);
>>> + qemu_mutex_unlock(&(queue->lock));
>>> + qemu_cond_signal(&(queue->cond));
>>>
>> In the case that we just spawned the threadlet, the cond_signal is
>> spurious. If we need predictable scheduling behaviour,
>> qemu_cond_signal needs to happen with queue->lock held.
>>
>
> It doesn't really affect predictability..
>
>> I'd rewrite the function as
>>
>> /**
>> * submit_threadletwork_to_queue: Submit a new task to a private queue to be
>> * executed asynchronously.
>> * @queue: Per-subsystem private queue to which the new task needs
>> * to be submitted.
>> * @work: Contains information about the task that needs to be submitted.
>> */
>> void submit_threadletwork_to_queue(ThreadletQueue *queue, ThreadletWork *work)
>> {
>> qemu_mutex_lock(&(queue->lock));
>> if (queue->idle_threads == 0&& (queue->cur_threads< queue->max_threads)) {
>> spawn_threadlet(queue);
>> } else {
>> qemu_cond_signal(&(queue->cond));
>> }
>> QTAILQ_INSERT_TAIL(&(queue->request_list), work, node);
>> qemu_mutex_unlock(&(queue->lock));
>> }
>>
>
> I think this is a lot more fragile. You're relying on the fact that signal will
> not cause the signalled thread to actually awaken until we release the lock and
> doing work after signalling that the signalled thread needs to be completed
> before it wakes up.
Given that qemu_cond_timedwait() need to get the queue->lock before returning
the singalled thread will wakeup and wait on the queue->lock.
- JV
>
> I think you're a lot more robust in the long term if you treat condition
> signalling as a hand off point because it makes the code a lot more explicit
> about what's happening.
>
>>> +/**
>>> + * submit_threadletwork: Submit to the global queue a new task to be executed
>>> + * asynchronously.
>>> + * @work: Contains information about the task that needs to be submitted.
>>> + */
>>> +void submit_threadletwork(ThreadletWork *work)
>>> +{
>>> + if (unlikely(!globalqueue_init)) {
>>> + threadlet_queue_init(&globalqueue, MAX_GLOBAL_THREADS,
>>> + MIN_GLOBAL_THREADS);
>>> + globalqueue_init = 1;
>>> + }
>>>
>> What protects globalqueue_init?
>>
>
> qemu_mutex, and that unlikely is almost certainly a premature optimization.
>
> Regards,
>
> Anthony Liguori
>
>
^ permalink raw reply
* [PATCH 1/1] Mx35pdk: enable debug board support
From: Fabio Estevam @ 2010-10-20 3:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTimdEhO9m7HW-TNxPaEVk85GDEThyn_DanJ_CZ+i@mail.gmail.com>
Xiao,
On Tue, Oct 19, 2010 at 11:45 PM, xiao jiang <jgq516@gmail.com> wrote:
>
> The patch is ok for me too,? and? the definition of EXPIO_PARENT_INT is
> similar like mx51_3ds board
> as I had said before.
> But since mxc_expio_init() should be return 0 if the debug board is
> detected, We should use
> mxc_expio_init(**_BASE_ADDR, EXPIO_PARENT_INT) other than
> !mxc_expio_init(**_BASE_ADDR, EXPIO_PARENT_INT), right?? The same issue also
> existed in mach-mx31_3ds.c.
mxc_expio_init returns 0 if the debug board is detected.
!mxc_expio_init returns 1 and print the error message.
So this is correct. I also get a message during the kernel boot saying
that the debug board was succesfully detected.
If you want, please resubmit a v2 of your original patch and also add my:
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Regards,
Fabio Esevam
^ permalink raw reply
* Re: Highlighting whitespace on removal with git diff
From: Kevin Ballard @ 2010-10-20 3:15 UTC (permalink / raw)
To: Stonky Fandango; +Cc: git
In-Reply-To: <AANLkTik7a8OQz2+SVTm+HjZkCtbjm6O9d12biCJ8MyZz@mail.gmail.com>
The highlight isn't telling you what changed on the line. It's telling you that this added line has trailing whitespace, and your core.whitespace config value is set such that this is considered an error.
-Kevin Ballard
On Oct 19, 2010, at 5:46 PM, Stonky Fandango wrote:
> When I do say 'git diff', and I have added a line with a trailing
> whitespace, i get a red square box highlighting the added whitespace.
> (on the '+' diff line)
> What I want, is the reverse - so when I delete an existing whitespace,
> I get something similar, for example a blue box highlighting the
> deleted whitespace (on the '-' diff line)
>
> Currently the only notificate I get is identical
> '-' and '+' diff lines, but no indication of the whitespace deleted
>
> I know I can do this with graphical tools, but ideally would like some way
> using the standard console git diff - if this is possible.
>
> I have the following in my .gitconfig
> ---
> [color "diff"]
> whitespace = red reverse
> [core]
> whitespace=-indent-with-non-tab,trailing-space,cr-at-eol,tab-in-indent
> ---
>
> Thanks,
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* virtio: console: Don't block entire guest if host doesn't read data
From: Rusty Russell @ 2010-10-20 3:15 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Amit Shah, Hans de Goede, virtualization
From: Amit Shah <amit.shah@redhat.com>
If the host is slow in reading data or doesn't read data at all,
blocking write calls not only blocked the program that called write()
but the entire guest itself.
To overcome this, let's not block till the host signals it has given
back the virtio ring element we passed it. Instead, send the buffer to
the host and return to userspace. This operation then becomes similar
to how non-blocking writes work, so let's use the existing code for this
path as well.
This code change also ensures blocking write calls do get blocked if
there's not enough room in the virtio ring as well as they don't return
-EAGAIN to userspace.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
CC: stable@kernel.org
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/char/virtio_console.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c810481..0f69c5e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -459,9 +459,12 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count,
/*
* Wait till the host acknowledges it pushed out the data we
- * sent. This is done for ports in blocking mode or for data
- * from the hvc_console; the tty operations are performed with
- * spinlocks held so we can't sleep here.
+ * sent. This is done for data from the hvc_console; the tty
+ * operations are performed with spinlocks held so we can't
+ * sleep here. An alternative would be to copy the data to a
+ * buffer and relax the spinning requirement. The downside is
+ * we need to kmalloc a GFP_ATOMIC buffer each time the
+ * console driver writes something out.
*/
while (!virtqueue_get_buf(out_vq, &len))
cpu_relax();
@@ -626,6 +629,14 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
goto free_buf;
}
+ /*
+ * We now ask send_buf() to not spin for generic ports -- we
+ * can re-use the same code path that non-blocking file
+ * descriptors take for blocking file descriptors since the
+ * wait is already done and we're certain the write will go
+ * through to the host.
+ */
+ nonblock = true;
ret = send_buf(port, buf, count, nonblock);
if (nonblock && ret > 0)
^ permalink raw reply related
* Re: [PATCH 1/3] IMA: move read/write counters into struct inode
From: Al Viro @ 2010-10-20 3:15 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Linus Torvalds, Eric Paris, linux-kernel, linux-security-module,
linux-fsdevel, hch, zohar, warthog9, david, jmorris, kyle, hpa,
akpm, mingo
In-Reply-To: <20101019191133.GE2447@parisc-linux.org>
On Tue, Oct 19, 2010 at 01:11:33PM -0600, Matthew Wilcox wrote:
> Hm. Sounds like the same question that the file leases code needs
> answered. The important difference is that the leases code can just
> refuse to set a lease on inodes with multiple dentries.
>
> While my mind's on it ... Al, is this code even close to correct?
>
> if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
> goto out;
> if ((arg == F_WRLCK)
> && ((atomic_read(&dentry->d_count) > 1)
> || (atomic_read(&inode->i_count) > 1)))
> goto out;
No. This is complete junk; note that e.g. ls -lR will disrupt it, since
lstat(2) will bump dentry refcount. The first part is more or less OK;
the second makes no sense.
What is it trying to do? Note that the first part also doesn't make a lot
of sense, since you could be acquiring a write reference *right* *now*,
just as that check passes. And you could finish getting it before you get
to do anything else in generic_setlease().
^ permalink raw reply
* Re: [patch 31/35] fs: icache per-zone inode LRU
From: KOSAKI Motohiro @ 2010-10-20 3:14 UTC (permalink / raw)
To: Dave Chinner; +Cc: kosaki.motohiro, npiggin, linux-kernel, linux-fsdevel
In-Reply-To: <20101019123852.GA12506@dastard>
Hello,
> On Tue, Oct 19, 2010 at 02:42:47PM +1100, npiggin@kernel.dk wrote:
> > Per-zone LRUs and shrinkers for inode cache.
>
> Regardless of whether this is the right way to scale or not, I don't
> like the fact that this moves the cache LRUs into the memory
> management structures, and expands the use of MM specific structures
> throughout the code. It ties the cache implementation to the current
> VM implementation. That, IMO, goes against all the principle of
> modularisation at the source code level, and it means we have to tie
> all shrinker implemenations to the current internal implementation
> of the VM. I don't think that is wise thing to do because of the
> dependencies and impedance mismatches it introduces.
>
> As an example: XFS inodes to be reclaimed are simply tagged in a
> radix tree so the shrinker can reclaim inodes in optimal IO order
> rather strict LRU order. It simply does not match a zone-based
> shrinker implementation in any way, shape or form, nor does it's
> inherent parallelism match that of the way shrinkers are called.
>
> Any change in shrinker infrastructure needs to be able to handle
> these sorts of impedance mismatches between the VM and the cache
> subsystem. The current API doesn't handle this very well, either,
> so it's something that we need to fix so that scalability is easy
> for everyone.
>
> Anyway, my main point is that tying the LRU and shrinker scaling to
> the implementation of the VM is a one-off solution that doesn't work
> for generic infrastructure. Other subsystems need the same
> large-machine scaling treatment, and there's no way we should be
> tying them all into the struct zone. It needs further abstraction.
I'm not sure what data structure is best. I can only say current
zone unawareness slab shrinker might makes following sad scenario.
o DMA zone shortage invoke and plenty icache in NORMAL zone dropping
o NUMA aware system enable zone_reclaim_mode, but shrink_slab() still
drop unrelated zone's icache
both makes performance degression. In other words, Linux does not have
flat memory model. so, I don't think Nick's basic concept is wrong.
It's straight forward enhancement. but if it don't fit current shrinkers,
I'd like to discuss how to make better data structure.
and I have dump question (sorry, I don't know xfs at all). current
xfs_mount is below.
typedef struct xfs_mount {
...
struct shrinker m_inode_shrink; /* inode reclaim shrinker */
} xfs_mount_t;
Do you mean xfs can't convert shrinker to shrinker[ZONES]? If so, why?
Thanks.
^ permalink raw reply
* Re: [PATCH 11/25] ASoC: Samsung: Add common I2S driver
From: Mark Brown @ 2010-10-20 3:13 UTC (permalink / raw)
To: Jassi Brar
Cc: alsa-devel, kgene.kim, Jassi Brar, ben-linux, june.bae, lrg,
sw.youn
In-Reply-To: <AANLkTimaQ=7Ejgb1jZW4=VGj2-ZV-2wcX6esZwUXsC4a@mail.gmail.com>
On Wed, Oct 20, 2010 at 11:22:26AM +0900, Jassi Brar wrote:
> On Tue, Oct 19, 2010 at 6:35 PM, Mark Brown
> > Might be as easy just to embed a struct s3c_audio_pdata within the
> > struct samsung_i2s?
> struct s3c_audio_pdata is shared among I2S, AC97, SPDIF and PCM.
> Here the idea is that controller drivers add specific structures to the union
> as and when they need it.
Right, what I'm saying is that it'd seem more natural to do things the
other way around and embed the common structure within a device specific
pdata structure instead of having the union.
> >> + if (is_secondary(i2s)) {
> >> + con |= CON_TXSDMA_ACTIVE;
> >> + con &= ~CON_TXSDMA_PAUSE;
> >> + } else {
> >> + con |= CON_TXDMA_ACTIVE;
> >> + con &= ~CON_TXDMA_PAUSE;
> > Can we do this stuff with a variable storing the mask to use? It might
> > compress the code a lot but I've not looked at what the bits actually
> > are.
> sorry, I am unable to understand what you suggest
If we have something in the driver data struct specifying the masks to
use then set these at probe time rather than having the if statements -
probably the same mask can be used for playback & record if the
bitfields are lined up similarly. This would then be:
con |= data->active_mask;
con &= ~data->pause_mask;
or similar, possibly with some shifts.
> >> + /* Allow LRCLK-inverted version of the supported formats */
> >> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> >> + case SND_SOC_DAIFMT_NB_NF:
> >> + break;
> >> + case SND_SOC_DAIFMT_NB_IF:
> >> + if (tmp & MOD_LR_RLOW)
> >> + tmp &= ~MOD_LR_RLOW;
> >> + else
> >> + tmp |= MOD_LR_RLOW;
> >> + break;
> > This looks a bit odd - it'll flip MOD_LR_FLOW if the frame is inverted,
> > I'd expect it to want to set a specific value?
> The frame polarity is specified by the Format(I2S, LSB, MSB), so we
> set/clear MOD_LR_RLOW acc to the Format requested.
> The Inversion request works relative to the Format -- if Frame inversion
> is requested, we simply flip it from the value set during Format setting.
Please add a comment explaining that you're inverting the orientation
you set previously - it's really surprising when reading the code.
> >> +static int i2sv2_i2s_set_clkdiv(struct snd_soc_dai *dai,
> >> + int div_id, int div)
> >> +{
> >> + struct i2s_dai *i2s = to_info(dai);
> >> + struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
> >> +
> >> + if (div_id == SAMSUNG_I2S_DIV_BCLK) {
> > switch statement please.
> Switch didn't look very pretty with just one case, so I made it if-else
> Anyways, I'll change it to switch.
It helps make the code correspond to the expected patterns for selecting
one of many clocks which is especially helpful when doing a big picture
scan.
> >> +/*
> >> + * Maximum number of I2S blocks that any SoC can have.
> >> + * The secondary interface of a CPU dai(if there exists any),
> >> + * is indexed at [cpu-dai's ID + MAX_I2S]
> >> + */
> >> +#define MAX_I2S 4
> > Can this be kept internal to the driver? If not it should be
> > namespaced.
> ASoC machine drivers need to index the secondary dai that is
> automatically created and registered by the cpu driver.
> So, it needs to be available outside.
> I'll make it SAMSUNG_I2S_SECOFF -- secondary offset of I2S(?)
How would the index be used with multi-component?
^ permalink raw reply
* Re: x86_32 compile error with the latest Mainline
From: Justin P. Mattock @ 2010-10-20 3:13 UTC (permalink / raw)
To: Linux Kernel Mailing List
In-Reply-To: <AANLkTi=kHWqg32TFVjX5GT+R2hDPcadxsvYhL7RXiGsp@mail.gmail.com>
On 10/19/2010 07:55 PM, Justin Mattock wrote:
> looking into upstart, I realize I need to have an image in order to
> load the policy
> at boot... so after learning something new(normally make, make install
> is just fine)
> I seem to be hitting some thing with dpkg-gencontrol
>
> make-kpkg --initrd --rootcmd fakeroot --revision=1
> --append-to-version -justin-rt kernel_image kernel_headers
> (probably have something wrong)
>
> and the error:
>
> -e 's/=ST/linux/g' -e 's/=R//g' \
> -e 's/=KPV/12.032/g' \
> -e 's/=K/vmlinuz/g' \
> -e 's@=MK@@g' -e 's@=A@i386@g' \
> -e 's/=I/YES/g' -e 's,=D,/boot,g' \
> -e 's/=MD//g' \
> -e 's@=M@@g' -e 's/=OF//g' \
> -e 's/=S//g' -e 's@=B@i386@g' \
> ./debian/templates.l10n> ./debian/templates.master
> install -p -o root -g root -m 644 ./debian/templates.master
> /home/kernel/2.6.37/debian/linux-image-2.6.36-rc8-justin-rt+/DEBIAN/templates
> dpkg-gencontrol -DArchitecture=i386 -isp \
> -plinux-image-2.6.36-rc8-justin-rt+
> -P/home/kernel/2.6.37/debian/linux-image-2.6.36-rc8-justin-rt+/
> dpkg-gencontrol: error: package linux-image-2.6.36-rc8-justin-rt+ not
> in control info
> make[2]: *** [debian/stamp/binary/linux-image-2.6.36-rc8-justin-rt+] Error 255
> make[2]: Leaving directory `/home/kernel/2.6.37'
> make[1]: *** [debian/stamp/binary/pre-linux-image-2.6.36-rc8-justin-rt+] Error 2
> make[1]: Leaving directory `/home/kernel/2.6.37'
> make: *** [kernel_image] Error 2
>
>
> I've gone in an enabled/disabled CONFIG_LOCALVERSION but still errors
> out both ways..
>
> any ideas on what I did wrong?
> BTW: also noticed this thread as well
> http://www.gossamer-threads.com/lists/linux/kernel/1266036
>
o.k. nevermind... needed to do more searching with this, ended up using
a patch on this report, that fixes this error for me:
https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.15/+bug/16447
Justin P. Mattock
^ permalink raw reply
* Re: [patch 31/35] fs: icache per-zone inode LRU
From: Nick Piggin @ 2010-10-20 3:12 UTC (permalink / raw)
To: Nick Piggin, linux-mm; +Cc: Dave Chinner, linux-kernel, linux-fsdevel
In-Reply-To: <20101020023556.GC3740@amd>
Gah. Try again.
On Wed, Oct 20, 2010 at 01:35:56PM +1100, Nick Piggin wrote:
> [I should have cc'ed this one to linux-mm as well, so I quote your
> reply in full here]
>
> On Tue, Oct 19, 2010 at 11:38:52PM +1100, Dave Chinner wrote:
> > On Tue, Oct 19, 2010 at 02:42:47PM +1100, npiggin@kernel.dk wrote:
> > > Per-zone LRUs and shrinkers for inode cache.
> >
> > Regardless of whether this is the right way to scale or not, I don't
> > like the fact that this moves the cache LRUs into the memory
> > management structures, and expands the use of MM specific structures
> > throughout the code.
>
> The zone structure really is the basic unit of memory abstraction
> in the whole zoned VM concept (which covers different properties
> of both physical address and NUMA cost).
>
> The zone contains structures for memory management that aren't
> otherwise directly related to one another. Generic page waitqueues,
> page allocator structures, pagecache reclaim structures, memory model
> data, and various statistics.
>
> Structures to reclaim inodes from a particular zone belong in the
> zone struct as much as those to reclaim pagecache or anonymous
> memory from that zone too. It actually fits far better in here than
> globally, because all our allocation/reclaiming/watermarks etc is
> driven per-zone.
>
> The structure is not frequent -- a couple per NUMA node.
>
>
> > It ties the cache implementation to the current
> > VM implementation. That, IMO, goes against all the principle of
> > modularisation at the source code level, and it means we have to tie
> > all shrinker implemenations to the current internal implementation
> > of the VM. I don't think that is wise thing to do because of the
> > dependencies and impedance mismatches it introduces.
>
> It's very fundamental. We allocate memory from, and have to reclaim
> memory from -- zones. Memory reclaim is driven based on how the VM
> wants to reclaim memory: nothing you can do to avoid some linkage
> between the two.
>
> Look at it this way. The dumb global shrinker is also tied to an
> MM implementation detail, but that detail in fact does *not* match
> the reality of the MM, and so it has all these problems interacting
> with real reclaim.
>
> What problems? OK, on an N zone system (assuming equal zones and
> even distribution of objects around memory), then if there is a shortage
> on a particular zone, slabs from _all_ zones are reclaimed. We reclaim
> a factor of N too many objects. In a NUMA situation, we also touch
> remote memory with a chance (N-1)/N.
>
> As number of nodes grow beyond 2, this quickly goes down hill.
>
> In summary, there needs to be some knowledge of how MM reclaims memory
> in memory reclaim shrinkers -- simply can't do a good implementation
> without that. If the zone concept changes, the MM gets turned upside
> down and all those assumptions would need to be revisited anyway.
>
>
> > As an example: XFS inodes to be reclaimed are simply tagged in a
> > radix tree so the shrinker can reclaim inodes in optimal IO order
> > rather strict LRU order. It simply does not match a zone-based
>
> This is another problem, similar to what we have in pagecache. In
> the pagecache, we need to clean pages in optimal IO order, but we
> still reclaim them according to some LRU order.
>
> If you reclaim them in optimal IO order, cache efficiency will go
> down because you sacrifice recency/frequency information. If you
> IO in reclaim order, IO efficiency goes down. The solution is to
> decouple them with like writeout versus reclaim.
>
> But anyway, that's kind of an "aside": inode caches are reclaimed
> in LRU, IO-suboptimal order today anyway. Per-zone LRU doesn't
> change that in the slightest.
>
> > shrinker implementation in any way, shape or form, nor does it's
> > inherent parallelism match that of the way shrinkers are called.
> >
> > Any change in shrinker infrastructure needs to be able to handle
> > these sorts of impedance mismatches between the VM and the cache
> > subsystem. The current API doesn't handle this very well, either,
> > so it's something that we need to fix so that scalability is easy
> > for everyone.
> >
> > Anyway, my main point is that tying the LRU and shrinker scaling to
> > the implementation of the VM is a one-off solution that doesn't work
> > for generic infrastructure.
>
> No it isn't. It worked for the pagecache, and it works for dcache.
>
>
> > Other subsystems need the same
> > large-machine scaling treatment, and there's no way we should be
> > tying them all into the struct zone. It needs further abstraction.
>
> An abstraction? Other than the zone? What do you suggest? Invent
> something that the VM has no concept of and try to use that?
>
> No. The zone is the right thing to base it on.
^ permalink raw reply
* Re: [patch 31/35] fs: icache per-zone inode LRU
From: Nick Piggin @ 2010-10-20 3:12 UTC (permalink / raw)
To: Nick Piggin, linux-mm; +Cc: Dave Chinner, linux-kernel, linux-fsdevel
In-Reply-To: <20101020023556.GC3740@amd>
Gah. Try again.
On Wed, Oct 20, 2010 at 01:35:56PM +1100, Nick Piggin wrote:
> [I should have cc'ed this one to linux-mm as well, so I quote your
> reply in full here]
>
> On Tue, Oct 19, 2010 at 11:38:52PM +1100, Dave Chinner wrote:
> > On Tue, Oct 19, 2010 at 02:42:47PM +1100, npiggin@kernel.dk wrote:
> > > Per-zone LRUs and shrinkers for inode cache.
> >
> > Regardless of whether this is the right way to scale or not, I don't
> > like the fact that this moves the cache LRUs into the memory
> > management structures, and expands the use of MM specific structures
> > throughout the code.
>
> The zone structure really is the basic unit of memory abstraction
> in the whole zoned VM concept (which covers different properties
> of both physical address and NUMA cost).
>
> The zone contains structures for memory management that aren't
> otherwise directly related to one another. Generic page waitqueues,
> page allocator structures, pagecache reclaim structures, memory model
> data, and various statistics.
>
> Structures to reclaim inodes from a particular zone belong in the
> zone struct as much as those to reclaim pagecache or anonymous
> memory from that zone too. It actually fits far better in here than
> globally, because all our allocation/reclaiming/watermarks etc is
> driven per-zone.
>
> The structure is not frequent -- a couple per NUMA node.
>
>
> > It ties the cache implementation to the current
> > VM implementation. That, IMO, goes against all the principle of
> > modularisation at the source code level, and it means we have to tie
> > all shrinker implemenations to the current internal implementation
> > of the VM. I don't think that is wise thing to do because of the
> > dependencies and impedance mismatches it introduces.
>
> It's very fundamental. We allocate memory from, and have to reclaim
> memory from -- zones. Memory reclaim is driven based on how the VM
> wants to reclaim memory: nothing you can do to avoid some linkage
> between the two.
>
> Look at it this way. The dumb global shrinker is also tied to an
> MM implementation detail, but that detail in fact does *not* match
> the reality of the MM, and so it has all these problems interacting
> with real reclaim.
>
> What problems? OK, on an N zone system (assuming equal zones and
> even distribution of objects around memory), then if there is a shortage
> on a particular zone, slabs from _all_ zones are reclaimed. We reclaim
> a factor of N too many objects. In a NUMA situation, we also touch
> remote memory with a chance (N-1)/N.
>
> As number of nodes grow beyond 2, this quickly goes down hill.
>
> In summary, there needs to be some knowledge of how MM reclaims memory
> in memory reclaim shrinkers -- simply can't do a good implementation
> without that. If the zone concept changes, the MM gets turned upside
> down and all those assumptions would need to be revisited anyway.
>
>
> > As an example: XFS inodes to be reclaimed are simply tagged in a
> > radix tree so the shrinker can reclaim inodes in optimal IO order
> > rather strict LRU order. It simply does not match a zone-based
>
> This is another problem, similar to what we have in pagecache. In
> the pagecache, we need to clean pages in optimal IO order, but we
> still reclaim them according to some LRU order.
>
> If you reclaim them in optimal IO order, cache efficiency will go
> down because you sacrifice recency/frequency information. If you
> IO in reclaim order, IO efficiency goes down. The solution is to
> decouple them with like writeout versus reclaim.
>
> But anyway, that's kind of an "aside": inode caches are reclaimed
> in LRU, IO-suboptimal order today anyway. Per-zone LRU doesn't
> change that in the slightest.
>
> > shrinker implementation in any way, shape or form, nor does it's
> > inherent parallelism match that of the way shrinkers are called.
> >
> > Any change in shrinker infrastructure needs to be able to handle
> > these sorts of impedance mismatches between the VM and the cache
> > subsystem. The current API doesn't handle this very well, either,
> > so it's something that we need to fix so that scalability is easy
> > for everyone.
> >
> > Anyway, my main point is that tying the LRU and shrinker scaling to
> > the implementation of the VM is a one-off solution that doesn't work
> > for generic infrastructure.
>
> No it isn't. It worked for the pagecache, and it works for dcache.
>
>
> > Other subsystems need the same
> > large-machine scaling treatment, and there's no way we should be
> > tying them all into the struct zone. It needs further abstraction.
>
> An abstraction? Other than the zone? What do you suggest? Invent
> something that the VM has no concept of and try to use that?
>
> No. The zone is the right thing to base it on.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH v2 4/4] builtins: check for startup_info->help, print and exit early
From: Nguyễn Thái Ngọc Duy @ 2010-10-20 3:12 UTC (permalink / raw)
To: git, Junio C Hamano, Jonathan Niedier
Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1287544320-8499-1-git-send-email-pclouds@gmail.com>
By ensuring no access to repo is done in "git foo -h" case, the commands
have better chance of really printing out help usage. Access to repo is not
necessary and may terminate program if it finds something wrong.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/branch.c | 3 +++
builtin/checkout-index.c | 3 +++
builtin/commit.c | 6 ++++++
builtin/gc.c | 3 +++
builtin/ls-files.c | 3 +++
builtin/merge.c | 3 +++
builtin/update-index.c | 3 +++
builtin/upload-archive.c | 7 ++++---
t/t3905-help.sh | 16 ++++++++--------
9 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 87976f0..9f152ed 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -667,6 +667,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+ if (startup_info->help)
+ usage_with_options(builtin_branch_usage, options);
+
git_config(git_branch_config, NULL);
if (branch_use_color == -1)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index a7a5ee1..7f25cd7 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -241,6 +241,9 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ if (startup_info->help)
+ usage_with_options(builtin_checkout_index_usage, builtin_checkout_index_options);
+
git_config(git_default_config, NULL);
state.base_dir = "";
prefix_length = prefix ? strlen(prefix) : 0;
diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..8b086f2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1070,6 +1070,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+ if (startup_info->help)
+ usage_with_options(builtin_status_usage, builtin_status_options);
+
if (null_termination && status_format == STATUS_FORMAT_LONG)
status_format = STATUS_FORMAT_PORCELAIN;
@@ -1255,6 +1258,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
int allow_fast_forward = 1;
struct wt_status s;
+ if (startup_info->help)
+ usage_with_options(builtin_commit_usage, builtin_commit_options);
+
wt_status_prepare(&s);
git_config(git_commit_config, &s);
in_merge = file_exists(git_path("MERGE_HEAD"));
diff --git a/builtin/gc.c b/builtin/gc.c
index c304638..5a9d0da 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -189,6 +189,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ if (startup_info->help)
+ usage_with_options(builtin_gc_usage, builtin_gc_options);
+
git_config(gc_config, NULL);
if (pack_refs < 0)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index bb4f612..814da51 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -530,6 +530,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
OPT_END()
};
+ if (startup_info->help)
+ usage_with_options(ls_files_usage, builtin_ls_files_options);
+
memset(&dir, 0, sizeof(dir));
prefix = cmd_prefix;
if (prefix)
diff --git a/builtin/merge.c b/builtin/merge.c
index 2dba3b9..0169694 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -917,6 +917,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list **remotes = &remoteheads;
+ if (startup_info->help)
+ usage_with_options(builtin_merge_usage, builtin_merge_options);
+
if (read_cache_unmerged()) {
die_resolve_conflict("merge");
}
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3ab214d..46a53f5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -589,6 +589,9 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
int lock_error = 0;
struct lock_file *lock_file;
+ if (startup_info->help)
+ usage(update_index_usage);
+
git_config(git_default_config, NULL);
/* We can't free this memory, it becomes part of a linked list parsed atexit() */
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 73f788e..d4f4741 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -26,9 +26,6 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
int sent_argc;
int len;
- if (argc != 2)
- usage(upload_archive_usage);
-
if (strlen(argv[1]) + 1 > sizeof(buf))
die("insanely long repository name");
@@ -98,6 +95,10 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
pid_t writer;
int fd1[2], fd2[2];
+
+ if (startup_info->help || argc != 2)
+ usage(upload_archive_usage);
+
/*
* Set up sideband subprocess.
*
diff --git a/t/t3905-help.sh b/t/t3905-help.sh
index 0dcbedf..6cab6b9 100755
--- a/t/t3905-help.sh
+++ b/t/t3905-help.sh
@@ -12,13 +12,13 @@ test_help() {
"
}
-test_help failure branch
-test_help failure checkout-index
-test_help failure commit
-test_help failure gc
-test_help failure ls-files
-test_help failure merge
-test_help failure update-index
-test_help failure upload-archive
+test_help success branch
+test_help success checkout-index
+test_help success commit
+test_help success gc
+test_help success ls-files
+test_help success merge
+test_help success update-index
+test_help success upload-archive
test_done
--
1.7.0.2.445.gcbdb3
^ permalink raw reply related
* [PATCH v2 3/4] builtins: utilize startup_info->help where possible
From: Nguyễn Thái Ngọc Duy @ 2010-10-20 3:11 UTC (permalink / raw)
To: git, Junio C Hamano, Jonathan Niedier
Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1287544320-8499-1-git-send-email-pclouds@gmail.com>
It helps reduce false alarms while I'm looking for "git foo -h" code
path that accesses repository. Anyway it looks like a good thing to
do. If one day people like to have "git foo --help" as an alternative
to "git foo -h", it would be easy.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/check-ref-format.c | 2 +-
builtin/grep.c | 2 +-
builtin/index-pack.c | 2 +-
builtin/log.c | 6 +-----
builtin/merge-ours.c | 2 +-
builtin/pack-redundant.c | 2 +-
builtin/show-ref.c | 2 +-
7 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index ae3f281..c6511e3 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -58,7 +58,7 @@ static int check_ref_format_print(const char *arg)
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage(builtin_check_ref_format_usage);
if (argc == 3 && !strcmp(argv[1], "--branch"))
diff --git a/builtin/grep.c b/builtin/grep.c
index da32f3d..ec39909 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -934,7 +934,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
* 'git grep -h', unlike 'git grep -h <pattern>', is a request
* to show usage information and exit.
*/
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage_with_options(grep_usage, options);
memset(&opt, 0, sizeof(opt));
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index e243d9d..649ad18 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -881,7 +881,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
struct pack_idx_entry **idx_objects;
unsigned char pack_sha1[20];
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage(index_pack_usage);
read_replace_refs = 0;
diff --git a/builtin/log.c b/builtin/log.c
index 22d1290..a7ba9ed 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -69,11 +69,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
if (default_date_mode)
rev->date_mode = parse_date_format(default_date_mode);
- /*
- * Check for -h before setup_revisions(), or "git log -h" will
- * fail when run without a git directory.
- */
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage(builtin_log_usage);
argc = setup_revisions(argc, argv, rev, opt);
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 6844116..8e0777b 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -20,7 +20,7 @@ static const char *diff_index_args[] = {
int cmd_merge_ours(int argc, const char **argv, const char *prefix)
{
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage(builtin_merge_ours_usage);
/*
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 41e1615..3f090b2 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -601,7 +601,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
unsigned char *sha1;
char buf[42]; /* 40 byte sha1 + \n + \0 */
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage(pack_redundant_usage);
for (i = 1; i < argc; i++) {
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index be9b512..7083fa9 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -204,7 +204,7 @@ static const struct option show_ref_options[] = {
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
- if (argc == 2 && !strcmp(argv[1], "-h"))
+ if (startup_info->help)
usage_with_options(show_ref_usage, show_ref_options);
argc = parse_options(argc, argv, prefix, show_ref_options,
--
1.7.0.2.445.gcbdb3
^ permalink raw reply related
* [PATCH v2 2/4] run_builtin(): save "-h" detection result for later use
From: Nguyễn Thái Ngọc Duy @ 2010-10-20 3:11 UTC (permalink / raw)
To: git, Junio C Hamano, Jonathan Niedier
Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1287544320-8499-1-git-send-email-pclouds@gmail.com>
When run_builtin() sees "-h" as the first argument, it assumes:
- this is the call for help usage
- the real git command will only print help usage then exit
So it skips all setup in this case. Unfortunately, some commands do
other things before calling parse_options(), which is often where the
help usage is printed. Some of those things may try to access the
repository unnecessarily. If a repository is broken, the command may
die() before it prints help usage, not really helpful.
Make real commands aware of this fast path so that they can handle it
properly (i.e., print help usage then exit immediately) if they were
going to do more initialization than git_config().
Demonstrating "git foo -h" fails depends on individual commands and
is generally difficult to do. Instead GIT_TRACE is used to check
if a command does set repo. If it does, it is supposed to fail if
repo setup code chokes.
"git upload-archive" fails for another reason, but will be fixed too
when "git upload-archive -h" is converted to use startup_info->help
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 1 +
git.c | 18 ++++++++++++++----
t/t3905-help.sh | 24 ++++++++++++++++++++++++
3 files changed, 39 insertions(+), 4 deletions(-)
create mode 100755 t/t3905-help.sh
diff --git a/cache.h b/cache.h
index 33decd9..bb57e34 100644
--- a/cache.h
+++ b/cache.h
@@ -1117,6 +1117,7 @@ const char *split_cmdline_strerror(int cmdline_errno);
/* git.c */
struct startup_info {
int have_repository;
+ int help; /* git foo -h */
};
extern struct startup_info *startup_info;
diff --git a/git.c b/git.c
index 50a1401..bb67540 100644
--- a/git.c
+++ b/git.c
@@ -246,13 +246,23 @@ struct cmd_struct {
static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
{
- int status, help;
+ int status;
struct stat st;
const char *prefix;
prefix = NULL;
- help = argc == 2 && !strcmp(argv[1], "-h");
- if (!help) {
+ startup_info->help = argc == 2 && !strcmp(argv[1], "-h");
+ if (startup_info->help) {
+ /*
+ * Fast path for "git foo -h", no setup is done.
+ * Other functions might set .git up automatically
+ * and potentially die() along the way. It's best
+ * to check this flag from the beginning, print its
+ * help usage and exit, nothing more.
+ */
+ ;
+ }
+ else {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
if (p->option & RUN_SETUP_GENTLY) {
@@ -267,7 +277,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
}
commit_pager_choice();
- if (!help && p->option & NEED_WORK_TREE)
+ if (!startup_info->help && p->option & NEED_WORK_TREE)
setup_work_tree();
trace_argv_printf(argv, "trace: built-in: git");
diff --git a/t/t3905-help.sh b/t/t3905-help.sh
new file mode 100755
index 0000000..0dcbedf
--- /dev/null
+++ b/t/t3905-help.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='tests that git foo -h should work even in potentially broken repos'
+
+. ./test-lib.sh
+
+test_help() {
+ test_expect_"$1" "$2 -h" "
+ GIT_TRACE=\"`pwd`\"/$2.log test_must_fail git $2 -h &&
+ test \$exit_code = 129 &&
+ ! grep 'defaults to' $2.log
+ "
+}
+
+test_help failure branch
+test_help failure checkout-index
+test_help failure commit
+test_help failure gc
+test_help failure ls-files
+test_help failure merge
+test_help failure update-index
+test_help failure upload-archive
+
+test_done
--
1.7.0.2.445.gcbdb3
^ permalink raw reply related
* [PATCH v2 1/4] setup_git_env: print the true $GIT_DIR for debugging
From: Nguyễn Thái Ngọc Duy @ 2010-10-20 3:11 UTC (permalink / raw)
To: git, Junio C Hamano, Jonathan Niedier
Cc: Nguyễn Thái Ngọc Duy
Ideally, GIT_DIR_ENVIRONMENT should be set before setup_git_env() is
called and "setup: GIT_DIR defaults to .git" should never be printed
out. If it is printed, $GIT_DIR is set up automatically, unexpectedly,
elsewhere.
Checking for that line is a good way to know if setup code works
properly.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Second try. Better tests.
environment.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/environment.c b/environment.c
index de5581f..e34adc3 100644
--- a/environment.c
+++ b/environment.c
@@ -91,8 +91,12 @@ static void setup_git_env(void)
git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
git_dir = git_dir ? xstrdup(git_dir) : NULL;
}
- if (!git_dir)
+ if (!git_dir) {
+ trace_printf("setup: GIT_DIR defaults to .git\n");
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+ }
+ else
+ trace_printf("setup: GIT_DIR set to %s\n", git_dir);
git_object_dir = getenv(DB_ENVIRONMENT);
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
--
1.7.0.2.445.gcbdb3
^ permalink raw reply related
* Re: [PATCH] staging:brcm80211:brcmfmac:change firmware/config file name
From: Greg KH @ 2010-10-20 3:10 UTC (permalink / raw)
To: Henry Ptasinski
Cc: Nohee Ko, devel@linuxdriverproject.org, Brett Rudley,
grundler@google.com, linux-wireless, dwmw2
In-Reply-To: <20101020021806.GM10832@broadcom.com>
On Tue, Oct 19, 2010 at 07:18:06PM -0700, Henry Ptasinski wrote:
> (Adding linux-wireless for input on firmware naming best practices.)
>
> On Tue, Oct 19, 2010 at 06:03:20PM -0700, Greg KH wrote:
> > On Tue, Oct 19, 2010 at 05:55:46PM -0700, Henry Ptasinski wrote:
> > > On Tue, Oct 19, 2010 at 05:47:43PM -0700, Greg KH wrote:
> > > > On Tue, Oct 19, 2010 at 04:45:26PM -0700, Nohee Ko wrote:
> > > > > >>
> > > > > Have you made the same changes in the linux-firmware source tree as
> > > > > well? Without changing it there, this could get messy.
> > > > > <<
> > > > >
> > > > > No not yet. But we are planning sooner or later.
> > > >
> > > > Please do it sooner.
> > > >
> > > > I'll wait for that to happen before committing this patch.
> > >
> > > There's no specific changes needed in the linux-firmware repo to work with this
> > > driver change. There is a bugfix version of the firmware that we're getting
> > > ready to send up, which is the motivation for this change, but the change
> > > will work properly with the firmware that's in the repo today.
> >
> > How, you just renamed the file you look for in the driver, right?
> >
> > > All this change does is replace the hard-coded full firmware filename used in
> > > the driver with the basename-apiversion construction as documented in
> > >
> > > http://www.linuxwireless.org/en/developers/Documentation/firmware-versioning
> > >
> > > plus add some info to the README to help users get the firmware properly
> > > installed.
> > >
> > > Users will need to create the symlinks as described in the README to use these
> > > driver changes.
> >
> > Yes, so you just broke their working setup, right?
> >
> > So, please get linux-firmware fixed to add the proper symlinks, and then
> > I can apply the patch, and say "go upgrade your linux-firmware package",
> > which is almost acceptable.
>
> Ah, I didn't realize we should have the symlinks in the linux-firmware repo.
> We didn't do that for the brcm80211 driver (same instructions to create
> symlinks by hand are in the README for that driver), and nobody pointed it out
> as being an issue.
>
> But I note that the linux-firmware repo doesn't contain any symlinks, and I'm
> having trouble finding any other driver that's using this approach. It looks
> like everybody is either just doing some variant of basename-apiversion, or
> hardcoding the complete firmware name (with codeversion) in theer driver.
>
> Should we just punt on the codeversion portion? Thoughts?
punt, as no one else has done this :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 11/18] fs: Introduce per-bucket inode hash locks
From: Nick Piggin @ 2010-10-20 3:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nick Piggin, Andi Kleen, Dave Chinner, linux-fsdevel,
linux-kernel, tglx
In-Reply-To: <20101019165044.GA13531@infradead.org>
On Tue, Oct 19, 2010 at 12:50:44PM -0400, Christoph Hellwig wrote:
> On Tue, Oct 19, 2010 at 06:00:57PM +1100, Nick Piggin wrote:
> > But it is still "magic". Because you don't even know whether it
> > is a spin or sleeping lock, let alone whether it is irq or bh safe.
> > You get far more information seeing a bit_spin_lock(0, &hlist) call
> > than hlist_lock().
> >
> > Even if you do rename them to hlist_bit_spin_lock, etc. Then you need
> > to add variants for each type of locking a caller wants to do on it.
> > Ask Linus what he thinks about that.
>
> And why do we need all these versions? We never had irqsave or bhsave
> versions of bitlocks. The only thing we might eventually want are
> trylock and is_locked operations once we grow user of it.
Or sleeping locks. And there is no reason not to have irqsave or
bhsave versions of bitlocks (which could enable interrupts etc on
a contended lock) but they just haven't been put in yet. If someone
needs them, and implements them in the bitlock code, everybody can
use them.
> To get back a bit to the point:
>
> - we have a new bl_hlist sturcture which combines a hash list and a
> lock embedded into the head
> - the reason why we do it is to be able to use a bitlock
To use bit 0 as a lock bit, yes. But if another use for the API was
required like to store some other OOB state (not a lock), then some
assertions can come out of the hlist_bl code quite easily and it
can be extended to have an arbitrary bit there. (or several arbitrary
bits if the pointer is aligned correctly).
> Now your initial version exposed the ugly defaults of that to the user
> which is required to case the hash head to and unsigned long and use
> bit_spin_lock on bit 0 of it. There's zero abstraction and a lot
> internal gutting required there.
What do you mean zero abstraction? It changes all the hlist operations
to handle bit 0 as lock bit.
Locking and unlocking a bit, we already have abstractions for. They work
really well, and nobody has to check the implementation to know what
they do.
^ permalink raw reply
* Re: [PATCH] net: make ctl_path local and const
From: Changli Gao @ 2010-10-20 3:10 UTC (permalink / raw)
To: Joe Perches
Cc: Andy Grover, linux-sctp, rds-devel, Pekka Savola (ipv6),
linux-x25, dccp, bridge, James Morris, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel, Ralf Baechle <ralf
In-Reply-To: <1287543701.10409.599.camel@Joe-Laptop>
On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> []
>> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
>> index 04e9c0d..b92f269 100644
>> --- a/net/appletalk/sysctl_net_atalk.c
>> +++ b/net/appletalk/sysctl_net_atalk.c
>> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
>> { },
>> };
>>
>> -static struct ctl_path atalk_path[] = {
>> - { .procname = "net", },
>> - { .procname = "appletalk", },
>> - { }
>> -};
>> -
>> static struct ctl_table_header *atalk_table_header;
>>
>> void atalk_register_sysctl(void)
>> {
>> + const struct ctl_path atalk_path[] = {
>
> Shouldn't all of these be static const struct ?
>
>
They needn't. And some variables are specified __net_initdata currently.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] net: make ctl_path local and const
From: Changli Gao @ 2010-10-20 3:10 UTC (permalink / raw)
To: Joe Perches
Cc: Andy Grover, linux-sctp, rds-devel, Pekka Savola (ipv6),
linux-x25, dccp, bridge, James Morris, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel, Ralf Baechle <ralf>
In-Reply-To: <1287543701.10409.599.camel@Joe-Laptop>
On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> []
>> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
>> index 04e9c0d..b92f269 100644
>> --- a/net/appletalk/sysctl_net_atalk.c
>> +++ b/net/appletalk/sysctl_net_atalk.c
>> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
>> { },
>> };
>>
>> -static struct ctl_path atalk_path[] = {
>> - { .procname = "net", },
>> - { .procname = "appletalk", },
>> - { }
>> -};
>> -
>> static struct ctl_table_header *atalk_table_header;
>>
>> void atalk_register_sysctl(void)
>> {
>> + const struct ctl_path atalk_path[] = {
>
> Shouldn't all of these be static const struct ?
>
>
They needn't. And some variables are specified __net_initdata currently.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [Bridge] [PATCH] net: make ctl_path local and const
From: Changli Gao @ 2010-10-20 3:10 UTC (permalink / raw)
To: Joe Perches
Cc: Andy Grover, linux-sctp, rds-devel, Pekka Savola (ipv6),
linux-x25, dccp, bridge, James Morris, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel, Ralf Baechle, David S. Miller,
netfilter-devel, Andrew Hendry
In-Reply-To: <1287543701.10409.599.camel@Joe-Laptop>
On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> []
>> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
>> index 04e9c0d..b92f269 100644
>> --- a/net/appletalk/sysctl_net_atalk.c
>> +++ b/net/appletalk/sysctl_net_atalk.c
>> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
>> { },
>> };
>>
>> -static struct ctl_path atalk_path[] = {
>> - { .procname = "net", },
>> - { .procname = "appletalk", },
>> - { }
>> -};
>> -
>> static struct ctl_table_header *atalk_table_header;
>>
>> void atalk_register_sysctl(void)
>> {
>> + const struct ctl_path atalk_path[] = {
>
> Shouldn't all of these be static const struct ?
>
>
They needn't. And some variables are specified __net_initdata currently.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* linux-next: manual merge of the swiotlb-xen tree with the pci tree
From: Stephen Rothwell @ 2010-10-20 3:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-next, linux-kernel, matt mooney, Jesse Barnes, Ryan Wilson
Hi Konrad,
Today's linux-next merge of the swiotlb-xen tree got a conflict in
drivers/pci/Makefile between commit
350a55e9ff6005032407d3234af800f413b03af5 ("PCI: use new ccflags variable
in Makefile") from the pci tree and commit
956a9202cd1220397933a07beda9f96b3df1fa24 ("xen-pcifront: Xen PCI frontend
driver") from the swiotlb-xen tree.
Just context changes. I fixed it up (see below) and can carry the fix as
necessary.
As I said in my other email, Konrad, you can ignore this report - it is
just for information. The conflict resolution is simple.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/pci/Makefile
index dcd7ace,d5e2705..0000000
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@@ -65,4 -65,8 +65,6 @@@ obj-$(CONFIG_PCI_SYSCALL) += syscall.
obj-$(CONFIG_PCI_STUB) += pci-stub.o
+ obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
+
-ifeq ($(CONFIG_PCI_DEBUG),y)
-EXTRA_CFLAGS += -DDEBUG
-endif
+ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
^ permalink raw reply
* Re: [patch 26/35] fs: icache alloc anonymous inode allocation
From: Nick Piggin @ 2010-10-20 3:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: npiggin, linux-kernel, linux-fsdevel
In-Reply-To: <20101019163327.GA5987@infradead.org>
On Tue, Oct 19, 2010 at 12:33:27PM -0400, Christoph Hellwig wrote:
> On Tue, Oct 19, 2010 at 02:42:42PM +1100, npiggin@kernel.dk wrote:
> > Provide new_anon_inode function for inodes without a default inode number, and
> > not on sb list. This can enable filesystems to reduce locking. "Real"
> > filesystems can also reduce locking by allocating anonymous inode first, then
> > adding it to lists after finding the inode number.
>
> Having an _anon inode allocation for fileystsem that do manage the inode
> lifetime is fine, but please don't mix that up with i_ino assignment,
> as they are two totally different things.
>
> Disk and network filesystem do not need a default i_ino, but they
> absolutely do no need their inodes to be on the per-sb list.
> anonfs/pipe/socket (and nothing else) can do away with the per-sb list,
> but they do need a pseudo inode number.
Probably bad wording of "anon". It should be "raw", maybe. The
filesystem is then of course responsible for adding i_ino and/or
to lists.
> I have a version of this port to Dave's tree which gets this right.
> i_ino assignment is already moved out by my patch (which should apply
> to your tree with minimal differences), so the new _anon only does not
> put the inode on the list. The other difference is that we don't bother
> initializing i_sb_list in the main inode allocation path, but only in
> new_anon_inode, and that the function is not exported - it really should
> only be used for built-in filesystems that never get unmounted to be
> safe.
I'll check it.
^ permalink raw reply
* [PATCH 2/2]x86: spread tlb flush vector between nodes
From: Shaohua Li @ 2010-10-20 3:07 UTC (permalink / raw)
To: lkml; +Cc: Ingo Molnar, hpa@zytor.com, Andi Kleen, Chen, Tim C
Currently flush tlb vector allocation is based on below equation:
sender = smp_processor_id() % 8
This isn't optimal, CPUs from different node can have the same vector, this
causes a lot of lock contention. Instead, we can assign the same vectors to
CPUs from the same node, while different node has different vectors. This has
below advantages:
a. if there is lock contention, the lock contention is between CPUs from one
node. This should be much cheaper than the contention between nodes.
b. completely avoid lock contention between nodes. This especially benefits
kswapd, which is the biggest user of tlb flush, since kswapd sets its affinity
to specific node.
In my test, this could reduce > 20% CPU overhead in extreme case.The test
machine has 4 nodes and each node has 16 CPUs. I then bind each node's kswapd
to the first CPU of the node. I run a workload with 4 sequential mmap file
read thread. The files are empty sparse file. This workload will trigger a
lot of page reclaim and tlbflush. The kswapd bind is to easy trigger the
extreme tlb flush lock contention because otherwise kswapd keeps migrating
between CPUs of a node and I can't get stable result. Sure in real workload,
we can't always see so big tlb flush lock contention, but it's possible.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
arch/x86/mm/tlb.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 47 insertions(+), 1 deletion(-)
Index: linux/arch/x86/mm/tlb.c
===================================================================
--- linux.orig/arch/x86/mm/tlb.c 2010-10-20 10:07:53.000000000 +0800
+++ linux/arch/x86/mm/tlb.c 2010-10-20 10:09:26.000000000 +0800
@@ -5,6 +5,7 @@
#include <linux/smp.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/cpu.h>
#include <asm/tlbflush.h>
#include <asm/mmu_context.h>
@@ -52,6 +53,8 @@ union smp_flush_state {
want false sharing in the per cpu data segment. */
static union smp_flush_state flush_state[NUM_INVALIDATE_TLB_VECTORS];
+static DEFINE_PER_CPU_READ_MOSTLY(int, tlb_vector_offset);
+
/*
* We cannot call mmdrop() because we are in interrupt context,
* instead update mm->cpu_vm_mask.
@@ -173,7 +176,7 @@ static void flush_tlb_others_ipi(const s
union smp_flush_state *f;
/* Caller has disabled preemption */
- sender = smp_processor_id() % NUM_INVALIDATE_TLB_VECTORS;
+ sender = per_cpu(tlb_vector_offset, smp_processor_id());
f = &flush_state[sender];
/*
@@ -218,6 +221,47 @@ void native_flush_tlb_others(const struc
flush_tlb_others_ipi(cpumask, mm, va);
}
+static void __cpuinit calculate_tlb_offset(void)
+{
+ int cpu, node, nr_node_vecs;
+ /*
+ * we are changing tlb_vector_offset for each CPU in runtime, but this
+ * will not cause inconsistency, as the write is atomic under X86. we
+ * might see more lock contentions in a short time, but after all CPU's
+ * tlb_vector_offset are changed, everything should go normal
+ *
+ * Note: if NUM_INVALIDATE_TLB_VECTORS % nr_online_nodes !=0, we might
+ * waste some vectors.
+ **/
+ if (nr_online_nodes > NUM_INVALIDATE_TLB_VECTORS)
+ nr_node_vecs = 1;
+ else
+ nr_node_vecs = NUM_INVALIDATE_TLB_VECTORS/nr_online_nodes;
+
+ for_each_online_node(node) {
+ int node_offset = (node % NUM_INVALIDATE_TLB_VECTORS) *
+ nr_node_vecs;
+ int cpu_offset = 0;
+ for_each_cpu(cpu, cpumask_of_node(node)) {
+ per_cpu(tlb_vector_offset, cpu) = node_offset +
+ cpu_offset;
+ cpu_offset++;
+ cpu_offset = cpu_offset % nr_node_vecs;
+ }
+ }
+}
+
+static int tlb_cpuhp_notify(struct notifier_block *n,
+ unsigned long action, void *hcpu)
+{
+ switch (action & 0xf) {
+ case CPU_ONLINE:
+ case CPU_DEAD:
+ calculate_tlb_offset();
+ }
+ return NOTIFY_OK;
+}
+
static int __cpuinit init_smp_flush(void)
{
int i;
@@ -225,6 +269,8 @@ static int __cpuinit init_smp_flush(void
for (i = 0; i < ARRAY_SIZE(flush_state); i++)
raw_spin_lock_init(&flush_state[i].tlbstate_lock);
+ calculate_tlb_offset();
+ hotcpu_notifier(tlb_cpuhp_notify, 0);
return 0;
}
core_initcall(init_smp_flush);
^ permalink raw reply
* [PATCH 1/2]percpu: introduce read mostly percpu API
From: Shaohua Li @ 2010-10-20 3:07 UTC (permalink / raw)
To: lkml; +Cc: Ingo Molnar, hpa@zytor.com, Andi Kleen, Chen, Tim C
Add a new readmostly percpu section and api, next patch will use it.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
include/asm-generic/vmlinux.lds.h | 4 ++++
include/linux/percpu-defs.h | 9 +++++++++
2 files changed, 13 insertions(+)
Index: linux/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux.orig/include/asm-generic/vmlinux.lds.h 2010-10-20 09:32:52.000000000 +0800
+++ linux/include/asm-generic/vmlinux.lds.h 2010-10-20 10:03:38.000000000 +0800
@@ -677,6 +677,8 @@
- LOAD_OFFSET) { \
VMLINUX_SYMBOL(__per_cpu_start) = .; \
*(.data..percpu..first) \
+ . = ALIGN(PAGE_SIZE); \
+ *(.data..percpu..readmostly) \
*(.data..percpu..page_aligned) \
*(.data..percpu) \
*(.data..percpu..shared_aligned) \
@@ -703,6 +705,8 @@
VMLINUX_SYMBOL(__per_cpu_load) = .; \
VMLINUX_SYMBOL(__per_cpu_start) = .; \
*(.data..percpu..first) \
+ . = ALIGN(PAGE_SIZE); \
+ *(.data..percpu..readmostly) \
*(.data..percpu..page_aligned) \
*(.data..percpu) \
*(.data..percpu..shared_aligned) \
Index: linux/include/linux/percpu-defs.h
===================================================================
--- linux.orig/include/linux/percpu-defs.h 2010-10-20 09:14:27.000000000 +0800
+++ linux/include/linux/percpu-defs.h 2010-10-20 09:17:08.000000000 +0800
@@ -139,6 +139,15 @@
__aligned(PAGE_SIZE)
/*
+ * Declaration/definition used for per-CPU variables that must be read mostly.
+ */
+#define DECLARE_PER_CPU_READ_MOSTLY(type, name) \
+ DECLARE_PER_CPU_SECTION(type, name, "..readmostly")
+
+#define DEFINE_PER_CPU_READ_MOSTLY(type, name) \
+ DEFINE_PER_CPU_SECTION(type, name, "..readmostly")
+
+/*
* Intermodule exports for per-CPU variables. sparse forgets about
* address space across EXPORT_SYMBOL(), change EXPORT_SYMBOL() to
* noop if __CHECKER__.
^ permalink raw reply
* Re: ib mad definitions
From: Jason Gunthorpe @ 2010-10-20 3:07 UTC (permalink / raw)
To: Ira Weiny
Cc: Hefty, Sean, Hal Rosenstock,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sasha Khapyorsky
In-Reply-To: <20101019183257.3f609c45.weiny2-i2BcT+NCU+M@public.gmane.org>
On Tue, Oct 19, 2010 at 06:32:57PM -0700, Ira Weiny wrote:
> On Tue, 19 Oct 2010 18:09:58 -0700
> Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
> > On Tue, Oct 19, 2010 at 06:00:51PM -0700, Hefty, Sean wrote:
> > > > Can we at least agree on the usage of these structures first? Are the
> > > > constants going to be in host or network byte order?
> > >
> > > I was simply suggesting to 'move' some of the existing structures and defines.
> >
> > But they are horrible and little used outside opensm right now, you
> > really want to commit to that forever?
> Not everything is horrible. And if it is we can fix it. But I think
> defining "yet another" header with the same functionality is worse.
> Like it or
libibumad is a system library. It needs to have a stable ABI, low
churn and ideally be 'complete'.
My database of IB structs has 117 structures, all with wakky alignment
and all manner of strangeness. IMHO, it is infeasible to keep with the
ad hoc approach in ibtypes.h and generate a complete header set
without a lot of churn. This is why it is horrible.
There are things worse than 'yet another' header - for instance a
system library being churned again and again for cleanups. Figure out
what you want, do it once, do it right, be done.
If we could all agree what these structs should look like I can
provide my database and someone can write the codegen AND WE CAN BE
DONE FOREVER. How is this not much better??
Don't treat the API of a system library as some casual thing. :(
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: fetch and bundle don't work in (semi-)broken repo
From: Nicolas Pitre @ 2010-10-20 3:06 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Uwe Kleine-König, git
In-Reply-To: <20101019210233.GA32029@burratino>
On Tue, 19 Oct 2010, Jonathan Nieder wrote:
> Nicolas Pitre wrote:
>
> > But ideally you should simply find a
> > pack that contains the problematic object in another repository and
> > copy it with its index file into the broken repository.
>
> I assume the object is gone for good, but if you have it in another
> repo that would be interesting, too.
>
> To be clear: I think the important data has been recovered from the
> broken repo already in the form of patches (right?) so the question
> at hand is whether it would be possible to teach git to do better at
> recovering automatically. Which might depend on the nature of the
> missing objects.
Sure. Given that it is possible to create a patch series, that means
that all the important objects are still available. Therefore Git
should be able to produce the pack for the equivalent fetch/bundle as
well.
So the following patch should help. I hope that Uwe still has a copy of
the broken repo to test this patch with.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f8eba53..691c2f1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1299,6 +1299,15 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
src->data = read_sha1_file(src_entry->idx.sha1, &type, &sz);
read_unlock();
if (!src->data)
+ if (src_entry->preferred_base) {
+ /*
+ * Those objects are not included in the
+ * resulting pack. Be resilient and ignore
+ * them, in case the pack could be created
+ * nevertheless.
+ */
+ return 0;
+ }
die("object %s cannot be read",
sha1_to_hex(src_entry->idx.sha1));
if (sz != src_size)
Nicolas
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.